diff --git a/client/src/main/java/org/asynchttpclient/uri/UriParser.java b/client/src/main/java/org/asynchttpclient/uri/UriParser.java index c65f145dd2..0665843138 100644 --- a/client/src/main/java/org/asynchttpclient/uri/UriParser.java +++ b/client/src/main/java/org/asynchttpclient/uri/UriParser.java @@ -15,6 +15,7 @@ */ package org.asynchttpclient.uri; +import io.netty.util.concurrent.FastThreadLocal; import org.jetbrains.annotations.Nullable; import static java.util.Objects.requireNonNull; @@ -22,6 +23,17 @@ final class UriParser { + // Reused per thread so a parse on a hot request path does not allocate a fresh parser each time. + // Safe because the sole caller (Uri.create) reads every field into a new Uri synchronously, right + // after parse() returns and before any other parse can run on the same thread — see reset() and the + // re-entrancy invariant note on parse(Uri, String). + private static final FastThreadLocal PARSER = new FastThreadLocal() { + @Override + protected UriParser initialValue() { + return new UriParser(); + } + }; + public @Nullable String scheme; public @Nullable String host; public int port = -1; @@ -31,11 +43,30 @@ final class UriParser { public String path = ""; public @Nullable String userInfo; - private final String originalUrl; + private String originalUrl = ""; private int start, end, currentIndex; - private UriParser(final String originalUrl) { + private UriParser() { + } + + /** + * Clear every field to its initial state and bind a new input. Must reset ALL fields — including the + * ones with non-null defaults ({@code port = -1}, {@code path = ""}) — so nothing bleeds from a prior + * parse when the per-thread instance is reused. + */ + private void reset(final String originalUrl) { this.originalUrl = originalUrl; + scheme = null; + host = null; + port = -1; + query = null; + fragment = null; + authority = null; + path = ""; + userInfo = null; + start = 0; + end = 0; + currentIndex = 0; } private void trimLeft() { @@ -368,9 +399,21 @@ private void parse(@Nullable Uri context) { computePath(queryOnly); } + /** + * Parse {@code originalUrl} (optionally resolved against {@code context}) and return the per-thread + * parser holding the result fields. + * + *

The returned instance is a thread-local scratch object reused across calls on the same thread, so + * the caller MUST read all needed fields before any other {@code parse} can run on the same thread. + * {@link Uri#create(Uri, String)} — the only caller — does exactly this: it copies every field into a + * new immutable {@link Uri} synchronously before returning, and the parse itself never re-enters + * {@code parse} (it only reads an already-built {@code context} Uri). Do not retain the returned + * instance or call {@code parse} re-entrantly within a single field-read sequence. + */ public static UriParser parse(@Nullable Uri context, final String originalUrl) { requireNonNull(originalUrl, "originalUrl"); - final UriParser parser = new UriParser(originalUrl); + final UriParser parser = PARSER.get(); + parser.reset(originalUrl); parser.parse(context); return parser; } diff --git a/client/src/test/java/org/asynchttpclient/uri/UriParserReuseTest.java b/client/src/test/java/org/asynchttpclient/uri/UriParserReuseTest.java new file mode 100644 index 0000000000..76af0344f3 --- /dev/null +++ b/client/src/test/java/org/asynchttpclient/uri/UriParserReuseTest.java @@ -0,0 +1,109 @@ +/* + * Copyright (c) 2026 AsyncHttpClient Project. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.asynchttpclient.uri; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Guards the {@link UriParser} per-thread scratch reuse: a parse must never see a field bled in from a + * previous parse on the same thread, and concurrent parses on different threads must not cross-contaminate. + */ +public class UriParserReuseTest { + + @Test + public void backToBackParseDoesNotBleedFields() { + // A rich URL populates every field (userInfo, port, query, fragment). + UriParser rich = UriParser.parse(null, "http://user:pass@host.example.com:8443/a/b?q=1#frag"); + assertEquals("host.example.com", rich.host); + assertEquals("user:pass", rich.userInfo); + assertEquals(8443, rich.port); + assertEquals("/a/b", rich.path); + assertEquals("q=1", rich.query); + assertEquals("frag", rich.fragment); + + // A sparse URL on the SAME thread reuses the scratch: every field the sparse URL does not set must + // be reset, not carry over from the rich parse. + UriParser sparse = UriParser.parse(null, "http://host2/"); + assertEquals("host2", sparse.host); + assertNull(sparse.userInfo, "userInfo must not bleed from the previous parse"); + assertEquals(-1, sparse.port, "port must be reset to -1, not bleed 8443"); + assertEquals("/", sparse.path); + assertNull(sparse.query, "query must not bleed"); + assertNull(sparse.fragment, "fragment must not bleed"); + } + + @Test + public void parseAfterSchemeRelativeDoesNotBleedScheme() { + UriParser https = UriParser.parse(null, "https://secure.example.com/x"); + assertEquals("https", https.scheme); + + // Bare host with no scheme: scheme must be null, not carry "https". + UriParser noScheme = UriParser.parse(null, "//other.example.com/y"); + assertNull(noScheme.scheme, "scheme must not bleed from the previous parse"); + assertEquals("other.example.com", noScheme.host); + } + + @Test + @Timeout(30) + public void concurrentParsesDoNotCrossContaminate() throws InterruptedException { + final int threads = 16; + final int iterations = 20_000; + ExecutorService pool = Executors.newFixedThreadPool(threads); + CountDownLatch start = new CountDownLatch(1); + CountDownLatch done = new CountDownLatch(threads); + AtomicInteger mismatches = new AtomicInteger(); + + for (int t = 0; t < threads; t++) { + final int id = t; + // Each thread always parses the SAME distinct URL; if the thread-local scratch ever leaked + // across threads, a thread would observe another thread's host/port. + final String host = "h" + id + ".example.com"; + final int port = 1000 + id; + final String url = "http://" + host + ':' + port + "/p" + id + "?k=" + id; + pool.execute(() -> { + try { + start.await(); + for (int i = 0; i < iterations; i++) { + UriParser p = UriParser.parse(null, url); + if (!host.equals(p.host) || p.port != port || !("k=" + id).equals(p.query)) { + mismatches.incrementAndGet(); + } + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } finally { + done.countDown(); + } + }); + } + + start.countDown(); + assertTrue(done.await(25, TimeUnit.SECONDS), "workers should finish in time"); + pool.shutdownNow(); + assertEquals(0, mismatches.get(), "no thread should observe another thread's parse fields"); + } +}