From afc4393ccccbfc29dbaaec0fa3e1bcf042e23bec Mon Sep 17 00:00:00 2001 From: Francesco Nigro Date: Tue, 10 Sep 2024 10:22:49 +0200 Subject: [PATCH] Save parsing twice numeric IPv4 address --- .../core/net/impl/HostAndPortBenchmark.java | 65 +++++++++ .../core/http/impl/Http1xServerRequest.java | 37 ++++- .../http/HttpServerRequestInternal.java | 6 + .../http/HttpServerRequestWrapper.java | 5 + .../vertx/core/net/impl/HostAndPortImpl.java | 135 +++++++++++++----- .../java/io/vertx/tests/http/Http1xTest.java | 3 + .../io/vertx/tests/net/HostAndPortTest.java | 10 ++ 7 files changed, 220 insertions(+), 41 deletions(-) create mode 100644 src/test/benchmarks/io/vertx/core/net/impl/HostAndPortBenchmark.java diff --git a/src/test/benchmarks/io/vertx/core/net/impl/HostAndPortBenchmark.java b/src/test/benchmarks/io/vertx/core/net/impl/HostAndPortBenchmark.java new file mode 100644 index 00000000000..6b0107b23c7 --- /dev/null +++ b/src/test/benchmarks/io/vertx/core/net/impl/HostAndPortBenchmark.java @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2011-2024 Contributors to the Eclipse Foundation + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 + * which is available at https://www.apache.org/licenses/LICENSE-2.0. + * + * SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 + */ + +package io.vertx.core.net.impl; + +import java.util.concurrent.TimeUnit; + +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; + +@State(Scope.Thread) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Warmup(iterations = 10, time = 200, timeUnit = TimeUnit.MILLISECONDS) +@Measurement(iterations = 10, time = 200, timeUnit = TimeUnit.MILLISECONDS) +@Fork(2) +public class HostAndPortBenchmark { + + @Param("192.168.0.1:8080") + private String host; + + @Setup + public void setup() { + } + + + @Benchmark + public int parseIPv4Address() { + String host = this.host; + return HostAndPortImpl.parseIPv4Address(host, 0, host.length()); + } + + @Benchmark + public int parseHost() { + String host = this.host; + return HostAndPortImpl.parseHost(host, 0, host.length()); + } + + @Benchmark + public HostAndPortImpl parseAuthority() { + return HostAndPortImpl.parseAuthority(host, -1); + } + + @Benchmark + public boolean isValidAuthority() { + return HostAndPortImpl.isValidAuthority(host); + } +} diff --git a/vertx-core/src/main/java/io/vertx/core/http/impl/Http1xServerRequest.java b/vertx-core/src/main/java/io/vertx/core/http/impl/Http1xServerRequest.java index f1c4dfb0b3e..f8050b1029a 100644 --- a/vertx-core/src/main/java/io/vertx/core/http/impl/Http1xServerRequest.java +++ b/vertx-core/src/main/java/io/vertx/core/http/impl/Http1xServerRequest.java @@ -33,6 +33,7 @@ import io.vertx.core.net.NetSocket; import io.vertx.core.net.SocketAddress; import io.vertx.core.internal.concurrent.InboundMessageQueue; +import io.vertx.core.net.impl.HostAndPortImpl; import io.vertx.core.spi.metrics.HttpServerMetrics; import io.vertx.core.spi.tracing.SpanKind; import io.vertx.core.spi.tracing.TagExtractor; @@ -51,6 +52,8 @@ */ public class Http1xServerRequest extends HttpServerRequestInternal implements io.vertx.core.spi.observability.HttpRequest { + private static final HostAndPort NULL_HOST_AND_PORT = HostAndPort.create("", -1); + private final Http1xServerConnection conn; final ContextInternal context; @@ -216,12 +219,38 @@ public String query() { } @Override - public synchronized HostAndPort authority() { + public boolean isValidAuthority() { + HostAndPort authority = this.authority; + if (authority == NULL_HOST_AND_PORT) { + return false; + } + if (authority != null) { + return true; + } + String host = getHeader(HttpHeaderNames.HOST); + if (host == null || !HostAndPortImpl.isValidAuthority(host)) { + this.authority = NULL_HOST_AND_PORT; + return false; + } + return true; + } + + @Override + public HostAndPort authority() { + HostAndPort authority = this.authority; + if (authority == NULL_HOST_AND_PORT) { + return null; + } if (authority == null) { String host = getHeader(HttpHeaderNames.HOST); - if (host != null) { - authority = HostAndPort.parseAuthority(host, -1); + if (host == null) { + this.authority = NULL_HOST_AND_PORT; + return null; } + // it's fine to have a benign race here as long as HostAndPort is immutable + // to ensure safe publication + authority = HostAndPort.parseAuthority(host, -1); + this.authority = authority; } return authority; } @@ -240,6 +269,7 @@ public Http1xServerResponse response() { @Override public MultiMap headers() { + MultiMap headers = this.headers; if (headers == null) { HttpHeaders reqHeaders = request.headers(); if (reqHeaders instanceof MultiMap) { @@ -247,6 +277,7 @@ public MultiMap headers() { } else { headers = new HeadersAdaptor(reqHeaders); } + this.headers = headers; } return headers; } diff --git a/vertx-core/src/main/java/io/vertx/core/internal/http/HttpServerRequestInternal.java b/vertx-core/src/main/java/io/vertx/core/internal/http/HttpServerRequestInternal.java index c7f6eca8b92..d07aa10e833 100644 --- a/vertx-core/src/main/java/io/vertx/core/internal/http/HttpServerRequestInternal.java +++ b/vertx-core/src/main/java/io/vertx/core/internal/http/HttpServerRequestInternal.java @@ -32,4 +32,10 @@ public abstract class HttpServerRequestInternal implements HttpServerRequest { */ public abstract Object metric(); + /** + * This method act as {@link #authority()}{@code != null}, trying to not allocated a new object if the authority is not yet parsed. + */ + public boolean isValidAuthority() { + return authority() != null; + } } diff --git a/vertx-core/src/main/java/io/vertx/core/internal/http/HttpServerRequestWrapper.java b/vertx-core/src/main/java/io/vertx/core/internal/http/HttpServerRequestWrapper.java index 58942d471d2..fccdf109467 100644 --- a/vertx-core/src/main/java/io/vertx/core/internal/http/HttpServerRequestWrapper.java +++ b/vertx-core/src/main/java/io/vertx/core/internal/http/HttpServerRequestWrapper.java @@ -110,6 +110,11 @@ public HostAndPort authority() { return delegate.authority(); } + @Override + public boolean isValidAuthority() { + return delegate.isValidAuthority(); + } + @Override public long bytesRead() { return delegate.bytesRead(); diff --git a/vertx-core/src/main/java/io/vertx/core/net/impl/HostAndPortImpl.java b/vertx-core/src/main/java/io/vertx/core/net/impl/HostAndPortImpl.java index d0afb9f0ef6..de9c7e084ff 100644 --- a/vertx-core/src/main/java/io/vertx/core/net/impl/HostAndPortImpl.java +++ b/vertx-core/src/main/java/io/vertx/core/net/impl/HostAndPortImpl.java @@ -1,9 +1,21 @@ package io.vertx.core.net.impl; +import java.util.Arrays; + import io.vertx.core.net.HostAndPort; public class HostAndPortImpl implements HostAndPort { + // digits lookup table to speed-up parsing + private static final byte[] DIGITS = new byte[128]; + + static { + Arrays.fill(DIGITS, (byte) -1); + for (int i = '0';i <= '9';i++) { + DIGITS[i] = (byte) (i - '0'); + } + } + public static int parseHost(String val, int from, int to) { int pos; if ((pos = parseIPLiteral(val, from, to)) != -1) { @@ -30,42 +42,60 @@ public static int parseIPv4Address(String s, int from, int to) { return -1; } } - return from < to && (from + 1 == s.length() || s.charAt(from + 1) != ':') ? -1 : from; + // from is the next position to parse: whatever come before is a valid IPv4 address + if (from == to) { + // we're done + return from; + } + assert from < to; + // we have more characters, let's check if it has enough space for a port + if (from + 1 == s.length()) { + // just a single character left, we don't care what it is + return -1; + } + // we have more characters + if (s.charAt(from) != ':') { + // we need : to start a port + return -1; + } + // we (maybe) have a port - even with a single digit; the ipv4 addr is fineFi + return from; } public static int parseDecOctet(String s, int from, int to) { int val = parseDigit(s, from++, to); - switch (val) { - case 0: - return from; - case 1: - case 2: - case 3: - case 4: - case 5: - case 6: - case 7: - case 8: - case 9: - int n = parseDigit(s, from, to); - if (n != -1) { - val = val * 10 + n; - n = parseDigit(s, ++from, to); - if (n != -1) { - from++; - val = val * 10 + n; - } - } - if (val < 256) { - return from; - } + if (val == 0) { + return from; + } + if (val < 0 || val > 9) { + return -1; + } + int n = parseDigit(s, from, to); + if (n != -1) { + val = val * 10 + n; + n = parseDigit(s, ++from, to); + if (n != -1) { + from++; + val = val * 10 + n; + } + } + if (val < 256) { + return from; } return -1; } private static int parseDigit(String s, int from, int to) { - char c; - return from < to && isDIGIT(c = s.charAt(from)) ? c - '0' : -1; + if (from >= to) { + return -1; + } + char ch = s.charAt(from); + // a very predictable condition + if (ch < 128) { + // negative short values are still positive ints + return DIGITS[ch]; + } + return -1; } public static int parseIPLiteral(String s, int from, int to) { @@ -96,7 +126,7 @@ private static boolean isALPHA(char ch) { } private static boolean isDIGIT(char ch) { - return ('0' <= ch && ch <= '9'); + return DIGITS[ch] != -1; } private static boolean isSubDelims(char ch) { @@ -107,6 +137,27 @@ static boolean isHEXDIG(char ch) { return isDIGIT(ch) || ('A' <= ch && ch <= 'F') || ('a' <= ch && ch <= 'f'); } + /** + * Validate an authority HTTP header, that is host [':' port]
+ * This method should behave like {@link #parseAuthority(String, int)}, + * but without the overhead of creating an object: when {@code true} + * {@code parseAuthority(s, -1)} should return a non-null value. + * + * @param s the string to parse + * @return {@code true} when the string is a valid authority + * @throws NullPointerException when the string is {@code null} + */ + public static boolean isValidAuthority(String s) { + int pos = parseHost(s, 0, s.length()); + if (pos == s.length()) { + return true; + } + if (pos < s.length() && s.charAt(pos) == ':') { + return parsePort(s, pos) != -1; + } + return false; + } + /** * Parse an authority HTTP header, that is host [':' port] * @param s the string to parse @@ -120,22 +171,30 @@ public static HostAndPortImpl parseAuthority(String s, int schemePort) { } if (pos < s.length() && s.charAt(pos) == ':') { String host = s.substring(0, pos); - int port = 0; - while (++pos < s.length()) { - int digit = parseDigit(s, pos, s.length()); - if (digit == -1) { - return null; - } - port = port * 10 + digit; - if (port > 65535) { - return null; - } + int port = parsePort(s, pos); + if (port == -1) { + return null; } return new HostAndPortImpl(host, port); } return null; } + private static int parsePort(String s, int pos) { + int port = 0; + while (++pos < s.length()) { + int digit = parseDigit(s, pos, s.length()); + if (digit == -1) { + return -1; + } + port = port * 10 + digit; + if (port > 65535) { + return -1; + } + } + return port; + } + private final String host; private final int port; diff --git a/vertx-core/src/test/java/io/vertx/tests/http/Http1xTest.java b/vertx-core/src/test/java/io/vertx/tests/http/Http1xTest.java index 221fe376409..96684b0b761 100644 --- a/vertx-core/src/test/java/io/vertx/tests/http/Http1xTest.java +++ b/vertx-core/src/test/java/io/vertx/tests/http/Http1xTest.java @@ -5499,6 +5499,8 @@ public void testEmptyHostPortionOfHostHeader() throws Exception { private void testEmptyHostPortionOfHostHeader(String hostHeader, int expectedPort) throws Exception { server.requestHandler(req -> { + assertEquals("", req.authority().host()); + assertTrue(((HttpServerRequestInternal) req).isValidAuthority()); assertEquals("", req.authority().host()); assertEquals(expectedPort, req.authority().port()); req.response().end(); @@ -5516,6 +5518,7 @@ private void testEmptyHostPortionOfHostHeader(String hostHeader, int expectedPor public void testMissingHostHeader() throws Exception { server.requestHandler(req -> { assertEquals(null, req.authority()); + assertFalse(((HttpServerRequestInternal) req).isValidAuthority()); testComplete(); }); startServer(testAddress); diff --git a/vertx-core/src/test/java/io/vertx/tests/net/HostAndPortTest.java b/vertx-core/src/test/java/io/vertx/tests/net/HostAndPortTest.java index 0ebc8043b47..098c1feb690 100644 --- a/vertx-core/src/test/java/io/vertx/tests/net/HostAndPortTest.java +++ b/vertx-core/src/test/java/io/vertx/tests/net/HostAndPortTest.java @@ -42,6 +42,7 @@ public void testParseIPV4Address() { assertEquals(-1, HostAndPortImpl.parseIPv4Address("10.0.0.1.nip.io", 0, 9)); assertEquals(8, HostAndPortImpl.parseIPv4Address("10.0.0.1.nip.io", 0, 8)); assertEquals(-1, HostAndPortImpl.parseIPv4Address("10.0.0.1:", 0, 9)); + assertEquals(8, HostAndPortImpl.parseIPv4Address("10.0.0.1:0", 0, 10)); } @Test @@ -60,12 +61,14 @@ public void testParseHost() { assertEquals(7, HostAndPortImpl.parseHost("0.0.0.0", 0, 7)); assertEquals(8, HostAndPortImpl.parseHost("10.0.0.1.nip.io", 0, 8)); assertEquals(15, HostAndPortImpl.parseHost("10.0.0.1.nip.io", 0, 15)); + assertEquals(8, HostAndPortImpl.parseHost("10.0.0.1:8080", 0, 15)); } @Test public void testParseHostAndPort() { assertHostAndPort("10.0.0.1.nip.io", -1, "10.0.0.1.nip.io"); assertHostAndPort("10.0.0.1.nip.io", 8443, "10.0.0.1.nip.io:8443"); + assertHostAndPort("127.0.0.1", 8080, "127.0.0.1:8080"); assertHostAndPort("example.com", 8080, "example.com:8080"); assertHostAndPort("example.com", -1, "example.com"); assertHostAndPort("0.1.2.3", -1, "0.1.2.3"); @@ -73,21 +76,28 @@ public void testParseHostAndPort() { assertHostAndPort("", -1, ""); assertHostAndPort("", 8080, ":8080"); assertNull(HostAndPortImpl.parseAuthority("/", -1)); + assertFalse(HostAndPortImpl.isValidAuthority("/")); assertNull(HostAndPortImpl.parseAuthority("10.0.0.1:x", -1)); + assertFalse(HostAndPortImpl.isValidAuthority("10.0.0.1:x")); } @Test public void testParseInvalid() { assertHostAndPort("localhost", 65535, "localhost:65535"); assertNull(HostAndPortImpl.parseAuthority("localhost:65536", -1)); + assertFalse(HostAndPortImpl.isValidAuthority("localhost:65536")); assertNull(HostAndPortImpl.parseAuthority("localhost:8080a", -1)); + assertFalse(HostAndPortImpl.isValidAuthority("localhost:8080a")); assertNull(HostAndPortImpl.parseAuthority("http://localhost:8080", -1)); + assertFalse(HostAndPortImpl.isValidAuthority("http://localhost:8080")); assertNull(HostAndPortImpl.parseAuthority("^", -1)); + assertFalse(HostAndPortImpl.isValidAuthority("^")); } private void assertHostAndPort(String expectedHost, int expectedPort, String actual) { HostAndPortImpl hostAndPort = HostAndPortImpl.parseAuthority(actual, -1); assertNotNull(hostAndPort); + assertTrue(HostAndPortImpl.isValidAuthority(actual)); assertEquals(expectedHost, hostAndPort.host()); assertEquals(expectedPort, hostAndPort.port()); }