From 41cfd9eab46a5e2f660a37bc0cddfc580c1f6962 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 29 Sep 2026 16:48:38 -0400 Subject: [PATCH 1/5] Avoid URISyntaxException for common bad input in HttpURLConnection URLs java.net.URL accepts strings that java.net.URI rejects (a space, |, {}, [] in the path, a stray % or a second #). HttpUrlConnectionDecorator and UrlConnectionDecorator called URL.toURI(), so every such request threw and reported a URISyntaxException and lost its http.url, peer and path-based resource tags. Add URIUtils.toURI(URL), which percent-encodes only the offending characters before parsing. Well-formed URLs go through unchanged and without extra allocation. Co-Authored-By: Claude Sonnet 5.5 --- .../decorator/UrlConnectionDecorator.java | 3 +- .../HttpUrlConnectionDecorator.java | 3 +- .../instrumentation/api/URIUtils.java | 115 ++++++++++++++++++ .../api/URIUtilsToURITest.java | 69 +++++++++++ 4 files changed, 188 insertions(+), 2 deletions(-) create mode 100644 internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/URIUtilsToURITest.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/UrlConnectionDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/UrlConnectionDecorator.java index a3c8be6dd5b..51ad60113e1 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/UrlConnectionDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/UrlConnectionDecorator.java @@ -6,6 +6,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.InternalSpanTypes; import datadog.trace.bootstrap.instrumentation.api.Tags; +import datadog.trace.bootstrap.instrumentation.api.URIUtils; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import java.net.URI; import java.net.URISyntaxException; @@ -56,7 +57,7 @@ public void onURI(@Nonnull AgentSpan span, @Nonnull URI uri) { public void onURL(@Nonnull final AgentSpan span, @Nonnull final URL url) { try { - onURI(span, url.toURI()); + onURI(span, URIUtils.toURI(url)); } catch (URISyntaxException e) { LOGGER.debug("Error tagging url", e); } diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/httpurlconnection/HttpUrlConnectionDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/httpurlconnection/HttpUrlConnectionDecorator.java index 8acf3578423..688b37c3cf0 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/httpurlconnection/HttpUrlConnectionDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/httpurlconnection/HttpUrlConnectionDecorator.java @@ -2,6 +2,7 @@ import static datadog.trace.api.cache.RadixTreeCache.UNSET_STATUS; +import datadog.trace.bootstrap.instrumentation.api.URIUtils; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.bootstrap.instrumentation.decorator.HttpClientDecorator; import java.net.HttpURLConnection; @@ -36,7 +37,7 @@ protected String method(final HttpURLConnection connection) { @Override protected URI url(final HttpURLConnection connection) throws URISyntaxException { - return connection.getURL().toURI(); + return URIUtils.toURI(connection.getURL()); } @Override diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/URIUtils.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/URIUtils.java index 5f1fd20504e..e45e69e1283 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/URIUtils.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/URIUtils.java @@ -5,6 +5,7 @@ import datadog.trace.api.iast.util.PropagationUtils; import java.net.MalformedURLException; import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; @@ -136,6 +137,120 @@ public static URI safeParse(final String unparsed) { } } + /** + * Converts a {@link URL} to a {@link URI} like {@link URL#toURI()}, but first percent-encodes the + * characters that {@link URI} rejects even though {@link URL} accepts them, so that a common kind + * of bad input does not cost a thrown and caught {@link URISyntaxException} on every request. + * + *

These are: a space or control character, any of {@code " < > \ ^ ` { | }}, a {@code [} or + * {@code ]} in the path, a {@code %} that is not followed by two hex digits, and a second {@code + * #}. Well-formed URLs are converted exactly as {@code url.toURI()} would, with no extra + * allocation. Other problems, such as an invalid scheme, still throw {@link URISyntaxException}. + */ + public static URI toURI(final URL url) throws URISyntaxException { + return new URI(escapeIllegalURIChars(url.toString())); + } + + /** Percent-encodes the characters {@link URI} rejects; returns the same string if none. */ + private static String escapeIllegalURIChars(final String s) { + final int length = s.length(); + final int pathStart = pathStart(s); + int pathEnd = length; + for (int i = pathStart; i < length; i++) { + final char c = s.charAt(i); + if (c == '?' || c == '#') { + pathEnd = i; + break; + } + } + + StringBuilder escaped = null; + boolean inFragment = false; + for (int i = 0; i < length; i++) { + final char c = s.charAt(i); + final boolean illegal; + if (c == '%') { + illegal = !isPercentEscape(s, i); + } else if (c == '#') { + illegal = inFragment; // only the first '#' starts the fragment + inFragment = true; + } else if (c == '[' || c == ']') { + illegal = i >= pathStart && i < pathEnd; // allowed in an IPv6 host, the query and fragment + } else { + illegal = isIllegalURIChar(c); + } + if (illegal) { + if (escaped == null) { + escaped = new StringBuilder(length + 16).append(s, 0, i); + } + appendPercentEncoded(escaped, c); + } else if (escaped != null) { + escaped.append(c); + } + } + return escaped == null ? s : escaped.toString(); + } + + /** Index where the path starts: after the authority of {@code scheme://authority}, or 0. */ + private static int pathStart(final String s) { + final int schemeEnd = s.indexOf("://"); + if (schemeEnd < 0) { + return 0; + } + for (int i = schemeEnd + 3; i < s.length(); i++) { + final char c = s.charAt(i); + if (c == '/' || c == '?' || c == '#') { + return i; + } + } + return s.length(); + } + + private static boolean isPercentEscape(final String s, final int i) { + return i + 2 < s.length() && isHexDigit(s.charAt(i + 1)) && isHexDigit(s.charAt(i + 2)); + } + + private static boolean isHexDigit(final char c) { + return (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F'); + } + + private static boolean isIllegalURIChar(final char c) { + if (c <= ' ' || c == 0x7F) { + return true; + } + switch (c) { + case '"': + case '<': + case '>': + case '\\': + case '^': + case '`': + case '{': + case '|': + case '}': + return true; + default: + // URI accepts other non-ASCII characters, but not spaces or control characters + return c >= 0x80 && (Character.isSpaceChar(c) || Character.isISOControl(c)); + } + } + + private static void appendPercentEncoded(final StringBuilder sb, final char c) { + if (c < 0x80) { + appendPercentEncoded(sb, (byte) c); + } else { + for (final byte b : String.valueOf(c).getBytes(StandardCharsets.UTF_8)) { + appendPercentEncoded(sb, b); + } + } + } + + private static void appendPercentEncoded(final StringBuilder sb, final byte b) { + sb.append('%') + .append(Character.toUpperCase(Character.forDigit((b >> 4) & 0xF, 16))) + .append(Character.toUpperCase(Character.forDigit(b & 0xF, 16))); + } + /** * Builds a lazily evaluated valid URL based on the scheme, host, port and path. * diff --git a/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/URIUtilsToURITest.java b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/URIUtilsToURITest.java new file mode 100644 index 00000000000..1234db3e2ca --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/URIUtilsToURITest.java @@ -0,0 +1,69 @@ +package datadog.trace.bootstrap.instrumentation.api; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.net.URISyntaxException; +import java.net.URL; +import org.junit.jupiter.api.Test; +import org.tabletest.junit.TableTest; + +@SuppressWarnings("deprecation") // new URL(String) is what applications use to build these URLs +class URIUtilsToURITest { + + @TableTest({ + "Scenario | Url | Expected ", + "well-formed | 'http://example.com/a/b?x=1#f' | 'http://example.com/a/b?x=1#f' ", + "valid escape kept | 'http://example.com/a%20b' | 'http://example.com/a%20b' ", + "space in path | 'http://example.com/a b' | 'http://example.com/a%20b' ", + "space in query | 'http://example.com/p?q=a b' | 'http://example.com/p?q=a%20b' ", + "pipe in query | 'http://example.com/p?q=a|b' | 'http://example.com/p?q=a%7Cb' ", + "braces in path | 'http://example.com/{id}' | 'http://example.com/%7Bid%7D' ", + "quotes and angle brackets | 'http://example.com/p?q=\"\"' | 'http://example.com/p?q=%22%3Cx%3E%22'", + "invalid escape | 'http://example.com/a%zz' | 'http://example.com/a%25zz' ", + "percent at end | 'http://example.com/100%' | 'http://example.com/100%25' ", + "truncated escape | 'http://example.com/a%4' | 'http://example.com/a%254' ", + "brackets in path | 'http://example.com/a[1]' | 'http://example.com/a%5B1%5D' ", + "brackets in query kept | 'http://example.com/p?ids[]=1' | 'http://example.com/p?ids[]=1' ", + "ipv6 host kept | 'http://[::1]:8080/x' | 'http://[::1]:8080/x' ", + "second hash | 'http://example.com/p#a#b' | 'http://example.com/p#a%23b' " + }) + void repairsUrlsThatUriRejects(String url, String expected) throws Exception { + assertEquals(expected, URIUtils.toURI(new URL(url)).toString()); + } + + @Test + void keepsNonAsciiLettersButEncodesNonAsciiSpaces() throws Exception { + final char eAcute = (char) 0xE9; + final char noBreakSpace = (char) 0xA0; + assertEquals( + "http://example.com/caf" + eAcute, + URIUtils.toURI(new URL("http://example.com/caf" + eAcute)).toString()); + assertEquals( + "http://example.com/a%C2%A0b", + URIUtils.toURI(new URL("http://example.com/a" + noBreakSpace + "b")).toString()); + } + + @Test + void convertsWellFormedUrlsLikeUrlToUri() throws Exception { + for (String text : + new String[] { + "http://example.com", + "https://user:pw@example.com:8443/a/b;c=d?x=1&y=2#frag", + "http://[::1]:8080/x", + "file:///tmp/some%20file.txt" + }) { + URL url = new URL(text); + assertEquals(url.toURI(), URIUtils.toURI(url), text); + } + } + + @Test + void stillThrowsForProblemsItDoesNotRepair() throws Exception { + // the multi-argument constructor does not validate the host, so this URL has an unterminated + // IPv6 host that java.net.URI rejects + URL url = new URL("http", "[::1", -1, "/x"); + + assertThrows(URISyntaxException.class, () -> URIUtils.toURI(url)); + } +} From 122aac91417724a48bbf4aef7b3220b88bd2b255 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 7 Oct 2026 15:55:29 -0400 Subject: [PATCH 2/5] Port AdaptiveLatch from #12672 Brought over unchanged, with its tests, for use in URL-to-URI conversion. Co-Authored-By: Claude Opus 5.5 --- .../datadog/trace/util/AdaptiveLatch.java | 124 +++++++++++ .../datadog/trace/util/AdaptiveLatchTest.java | 192 ++++++++++++++++++ 2 files changed, 316 insertions(+) create mode 100644 internal-api/src/main/java/datadog/trace/util/AdaptiveLatch.java create mode 100644 internal-api/src/test/java/datadog/trace/util/AdaptiveLatchTest.java diff --git a/internal-api/src/main/java/datadog/trace/util/AdaptiveLatch.java b/internal-api/src/main/java/datadog/trace/util/AdaptiveLatch.java new file mode 100644 index 00000000000..96e5408d957 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/util/AdaptiveLatch.java @@ -0,0 +1,124 @@ +package datadog.trace.util; + +import javax.annotation.Nullable; +import javax.annotation.concurrent.ThreadSafe; + +/** + * A self-resetting switch between two ways of performing an operation whose failure depends on the + * input, such as parsing a value from a producer that sometimes sends garbage: an optimistic path, + * {@link #apply}, that is fastest on good input but throws {@code X} on bad input, and a cautious + * path, {@link #applySafely}, that never throws {@code X}. Unlike {@code Latch} and {@link + * ClassLatch}, it never skips the operation: a bad input says nothing about the next one. + * + *

Disengaged, every call takes the optimistic path. A failure there engages the latch, and from + * then on calls take the cautious path until {@code closeAfter} consecutive inputs pass it cleanly. + * The cautious path reports bad input by returning {@link #reject}, which restarts that count; + * anything else it returns counts as clean. + * + *

The cautious path can be built in several ways: + * + *

+ * + *

Choosing {@code closeAfter} is a rent-or-buy decision. Staying engaged costs the cautious + * path's overhead on every good input; disengaging costs one optimistic failure when the next bad + * input arrives. Disengaging once the accumulated overhead would match one failure, that is after + * about {@code failureCost / cautiousOverhead} good inputs, is never worse than twice the best + * possible schedule, whatever the input. Measure both costs at a realistic stack depth: the cost of + * a throw grows with the stack it fills in. + * + *

Intended as a {@code static final} field of a named final subclass, one per call site: the + * receiver is then a constant of a known exact type, so the JIT can inline the hooks. Disengaged, + * the latch adds one plain field read to the optimistic path. + * + *

This is a hint, not a lock. The state is a plain counter, and it counts calls, not time. A + * stale or lost update only costs one more cautious call or one more optimistic failure, never a + * wrong result. + * + * @param the type of the value the operation is applied to + * @param the type of the result + * @param the failure that engages the latch; anything else propagates unchanged + */ +@ThreadSafe +public abstract class AdaptiveLatch { + private final Class failureType; + private final int closeAfter; + + /** 0 when disengaged; otherwise the clean calls left before disengaging. */ + private int remaining; + + /** + * @param failureType the failure of {@link #apply} that engages the latch + * @param closeAfter how many consecutive clean calls disengage it, at least 1; see the class + * comment for how to choose it + */ + protected AdaptiveLatch(Class failureType, int closeAfter) { + if (closeAfter < 1) { + throw new IllegalArgumentException("closeAfter must be at least 1: " + closeAfter); + } + this.failureType = failureType; + this.closeAfter = closeAfter; + } + + /** The optimistic path: fastest on good input, but may throw {@code X} for bad input. */ + @Nullable + protected abstract R apply(T input); + + /** + * The cautious path: must not throw {@code X}. Returns {@link #reject} for bad input; any other + * return counts as clean, including {@code null}. + */ + @Nullable + protected abstract R applySafely(T input); + + /** How many consecutive clean calls disengage the latch. */ + public final int closeAfter() { + return closeAfter; + } + + /** What a rejected input yields. {@code null} unless overridden. */ + @Nullable + protected R fallback(T input) { + return null; + } + + /** Reports bad input from {@link #applySafely}: restarts the count, and returns the fallback. */ + @Nullable + protected final R reject(T input) { + remaining = closeAfter; + return fallback(input); + } + + /** + * Performs the operation: optimistically while disengaged, cautiously while engaged. An + * optimistic failure engages the latch, and the same input is then retried cautiously, so a + * cautious path that can repair it still gets the chance. + */ + @Nullable + public final R tryApply(T input) { + final int remaining = this.remaining; + if (remaining > 0) { + // counted as clean up front; reject() re-arms the count if it is not + this.remaining = remaining - 1; + return applySafely(input); + } + try { + return apply(input); + } catch (RuntimeException e) { + if (!failureType.isInstance(e)) { + throw e; + } + this.remaining = closeAfter; + return applySafely(input); + } + } + + /** Returns whether calls currently take the cautious path. */ + public final boolean isEngaged() { + return remaining > 0; + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/AdaptiveLatchTest.java b/internal-api/src/test/java/datadog/trace/util/AdaptiveLatchTest.java new file mode 100644 index 00000000000..cf8422325e0 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/AdaptiveLatchTest.java @@ -0,0 +1,192 @@ +package datadog.trace.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; + +class AdaptiveLatchTest { + + /** + * Parses an int. The cautious path accepts only plain digits and rejects anything else, so + * "sneaky" input such as an overflowing number passes neither path; "1_000" is repaired by the + * cautious path, which the optimistic path rejects. + */ + private static final class Parsing extends AdaptiveLatch { + final AtomicInteger optimistic = new AtomicInteger(); + final AtomicInteger cautious = new AtomicInteger(); + + Parsing() { + super(NumberFormatException.class, 3); + } + + @Override + protected Integer apply(String input) { + optimistic.incrementAndGet(); + if ("boom".equals(input)) { + throw new IllegalStateException("boom"); + } + return Integer.parseInt(input); + } + + @Override + protected Integer applySafely(String input) { + cautious.incrementAndGet(); + String digits = input.replace("_", ""); + if (digits.isEmpty() || digits.length() > 9) { + return reject(input); + } + int value = 0; + for (int i = 0; i < digits.length(); i++) { + char c = digits.charAt(i); + if (c < '0' || c > '9') { + return reject(input); + } + value = value * 10 + (c - '0'); + } + return value; + } + } + + @Test + void disengagedItTakesOnlyTheOptimisticPath() { + Parsing latch = new Parsing(); + + assertEquals(42, latch.tryApply("42")); + assertEquals(7, latch.tryApply("7")); + + assertEquals(2, latch.optimistic.get()); + assertEquals(0, latch.cautious.get()); + assertFalse(latch.isEngaged()); + } + + @Test + void anOptimisticFailureEngagesAndRetriesTheSameInputCautiously() { + Parsing latch = new Parsing(); + + assertNull(latch.tryApply("bad")); + + assertEquals(1, latch.optimistic.get()); + assertEquals(1, latch.cautious.get()); + assertTrue(latch.isEngaged()); + } + + @Test + void theCautiousRetryCanRepairWhatTheOptimisticPathRejected() { + Parsing latch = new Parsing(); + + assertEquals(1000, latch.tryApply("1_000")); + assertTrue(latch.isEngaged()); + } + + @Test + void engagedItTakesOnlyTheCautiousPath() { + Parsing latch = new Parsing(); + latch.tryApply("bad"); + + assertEquals(42, latch.tryApply("42")); + assertNull(latch.tryApply("bad")); + + assertEquals(1, latch.optimistic.get(), "no optimistic call, and so no throw, while engaged"); + assertEquals(3, latch.cautious.get()); + } + + @Test + void disengagesAfterEnoughConsecutiveCleanCalls() { + Parsing latch = new Parsing(); + latch.tryApply("bad"); + + latch.tryApply("1"); + latch.tryApply("2"); + assertTrue(latch.isEngaged()); + latch.tryApply("3"); + assertFalse(latch.isEngaged()); + + assertEquals(4, latch.tryApply("4")); + assertEquals(2, latch.optimistic.get(), "disengaged again, so back on the optimistic path"); + } + + @Test + void aRejectionWhileEngagedRestartsTheCount() { + Parsing latch = new Parsing(); + latch.tryApply("bad"); + latch.tryApply("1"); + latch.tryApply("2"); + + assertNull(latch.tryApply("bad")); + + latch.tryApply("3"); + latch.tryApply("4"); + assertTrue(latch.isEngaged(), "the count restarted from closeAfter"); + latch.tryApply("5"); + assertFalse(latch.isEngaged()); + } + + @Test + void otherExceptionsPropagateWithoutEngaging() { + Parsing latch = new Parsing(); + + assertThrows(IllegalStateException.class, () -> latch.tryApply("boom")); + assertFalse(latch.isEngaged()); + assertEquals(0, latch.cautious.get()); + } + + @Test + void rejectYieldsTheFallbackAndARealNullCountsAsClean() { + AdaptiveLatch latch = + new AdaptiveLatch( + IllegalArgumentException.class, 1) { + @Override + protected String apply(String input) { + if (input.isEmpty()) { + throw new IllegalArgumentException(); + } + return "null".equals(input) ? null : input; + } + + @Override + protected String applySafely(String input) { + if (input.isEmpty()) { + return reject(input); + } + return "null".equals(input) ? null : input; + } + + @Override + protected String fallback(String input) { + return "fallback"; + } + }; + + // failed optimistically, then rejected cautiously + assertEquals("fallback", latch.tryApply("")); + // rejected while engaged + assertEquals("fallback", latch.tryApply("")); + // a cautious call that succeeds with null is clean, and with closeAfter 1 disengages + assertNull(latch.tryApply("null")); + assertFalse(latch.isEngaged()); + } + + @Test + void closeAfterMustBeAtLeastOne() { + assertThrows( + IllegalArgumentException.class, + () -> + new AdaptiveLatch( + IllegalArgumentException.class, 0) { + @Override + protected String apply(String input) { + return input; + } + + @Override + protected String applySafely(String input) { + return input; + } + }); + } +} From 10a3020169e17d9e1bfff394cd741a81b55783ca Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 7 Oct 2026 15:56:50 -0400 Subject: [PATCH 3/5] Let AdaptiveLatch take checked failures and report repaired input X may now be a checked exception, as with ClassLatch, so a latch can guard URI parsing. repaired(result) restarts the engaged count like reject but keeps the repaired result, so a steady stream of repairable input stays on the cautious path instead of paying one optimistic failure every closeAfter calls. Co-Authored-By: Claude Opus 5.5 --- .../datadog/trace/util/AdaptiveLatch.java | 40 +++++++++--- .../datadog/trace/util/AdaptiveLatchTest.java | 63 +++++++++++++++++++ 2 files changed, 95 insertions(+), 8 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/util/AdaptiveLatch.java b/internal-api/src/main/java/datadog/trace/util/AdaptiveLatch.java index 96e5408d957..6124b8caebf 100644 --- a/internal-api/src/main/java/datadog/trace/util/AdaptiveLatch.java +++ b/internal-api/src/main/java/datadog/trace/util/AdaptiveLatch.java @@ -1,5 +1,6 @@ package datadog.trace.util; +import java.lang.reflect.UndeclaredThrowableException; import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; @@ -12,8 +13,9 @@ * *

Disengaged, every call takes the optimistic path. A failure there engages the latch, and from * then on calls take the cautious path until {@code closeAfter} consecutive inputs pass it cleanly. - * The cautious path reports bad input by returning {@link #reject}, which restarts that count; - * anything else it returns counts as clean. + * The cautious path reports bad input by returning {@link #reject}, or {@link #repaired} when it + * still produced a result, either of which restarts that count; anything else it returns counts as + * clean. * *

The cautious path can be built in several ways: * @@ -21,7 +23,8 @@ *

  • an exception-free implementation of the same operation; *
  • a cheap, correct pre-check in front of the optimistic path: {@code return isKnownBad(input) * ? reject(input) : apply(input);} - *
  • a repair that turns common bad input into good input before the optimistic path. + *
  • a repair that turns common bad input into good input before the optimistic path, reporting + * each input it had to repair through {@link #repaired}. * * *

    Choosing {@code closeAfter} is a rent-or-buy decision. Staying engaged costs the cautious @@ -41,10 +44,10 @@ * * @param the type of the value the operation is applied to * @param the type of the result - * @param the failure that engages the latch; anything else propagates unchanged + * @param the failure that engages the latch, checked or not; anything else propagates unchanged */ @ThreadSafe -public abstract class AdaptiveLatch { +public abstract class AdaptiveLatch { private final Class failureType; private final int closeAfter; @@ -66,7 +69,7 @@ protected AdaptiveLatch(Class failureType, int closeAfter) { /** The optimistic path: fastest on good input, but may throw {@code X} for bad input. */ @Nullable - protected abstract R apply(T input); + protected abstract R apply(T input) throws X; /** * The cautious path: must not throw {@code X}. Returns {@link #reject} for bad input; any other @@ -93,6 +96,17 @@ protected final R reject(T input) { return fallback(input); } + /** + * Reports bad input that {@link #applySafely} repaired: restarts the count, like {@link #reject}, + * but returns the repaired result. Without it, a steady stream of repairable input would count as + * clean, disengage the latch, and pay one optimistic failure every {@code closeAfter} calls. + */ + @Nullable + protected final R repaired(@Nullable R result) { + remaining = closeAfter; + return result; + } + /** * Performs the operation: optimistically while disengaged, cautiously while engaged. An * optimistic failure engages the latch, and the same input is then retried cautiously, so a @@ -112,11 +126,21 @@ public final R tryApply(T input) { if (!failureType.isInstance(e)) { throw e; } - this.remaining = closeAfter; - return applySafely(input); + return engage(input); + } catch (Exception e) { + // apply declares only X, so any checked exception here is X unless it was thrown sneakily + if (!failureType.isInstance(e)) { + throw new UndeclaredThrowableException(e); + } + return engage(input); } } + private R engage(T input) { + this.remaining = closeAfter; + return applySafely(input); + } + /** Returns whether calls currently take the cautious path. */ public final boolean isEngaged() { return remaining > 0; diff --git a/internal-api/src/test/java/datadog/trace/util/AdaptiveLatchTest.java b/internal-api/src/test/java/datadog/trace/util/AdaptiveLatchTest.java index cf8422325e0..3887c391860 100644 --- a/internal-api/src/test/java/datadog/trace/util/AdaptiveLatchTest.java +++ b/internal-api/src/test/java/datadog/trace/util/AdaptiveLatchTest.java @@ -6,6 +6,8 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.net.URI; +import java.net.URISyntaxException; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; @@ -189,4 +191,65 @@ protected String applySafely(String input) { } }); } + + /** + * Parses a URI, a checked failure. The cautious path repairs spaces and reports each repair, so + * repairable input keeps the latch engaged. + */ + private static final class UriParsing extends AdaptiveLatch { + final AtomicInteger optimistic = new AtomicInteger(); + + UriParsing() { + super(URISyntaxException.class, 2); + } + + @Override + protected URI apply(String input) throws URISyntaxException { + optimistic.incrementAndGet(); + return new URI(input); + } + + @Override + protected URI applySafely(String input) { + String escaped = input.replace(" ", "%20"); + try { + URI uri = new URI(escaped); + return escaped.equals(input) ? uri : repaired(uri); + } catch (URISyntaxException e) { + return reject(input); + } + } + } + + @Test + void aCheckedFailureEngagesTheLatch() { + UriParsing latch = new UriParsing(); + + assertEquals(URI.create("/a%20b"), latch.tryApply("/a b")); + assertTrue(latch.isEngaged()); + } + + @Test + void repairedInputKeepsTheLatchEngaged() { + UriParsing latch = new UriParsing(); + latch.tryApply("/a b"); + + for (int i = 0; i < 5; i++) { + assertEquals(URI.create("/c%20d"), latch.tryApply("/c d")); + } + + assertTrue(latch.isEngaged(), "each repair restarts the count"); + assertEquals(1, latch.optimistic.get(), "only the first bad input reached the optimistic path"); + } + + @Test + void cleanInputAfterRepairsStillDisengages() { + UriParsing latch = new UriParsing(); + latch.tryApply("/a b"); + + latch.tryApply("/clean"); + latch.tryApply("/clean"); + + assertFalse(latch.isEngaged()); + } } From b3b464086d893c9ebf3b5746cbbc0e5275b40741 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 7 Oct 2026 15:59:19 -0400 Subject: [PATCH 4/5] Repair URLs only while bad ones are arriving URIUtils.toURI now goes through an AdaptiveLatch: well-formed URLs take url.toURI() directly and skip the scan, and the repairing conversion runs only after a URISyntaxException, until 64 well-formed URLs in a row. A repaired URL keeps the latch engaged. Unrepairable URLs still throw URISyntaxException. Co-Authored-By: Claude Opus 5.5 --- .../instrumentation/api/URIUtils.java | 50 +++++++++++++++++-- .../api/URIUtilsToURITest.java | 46 +++++++++++++++++ 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/URIUtils.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/URIUtils.java index e45e69e1283..be22366f243 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/URIUtils.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/URIUtils.java @@ -3,6 +3,7 @@ import static datadog.trace.api.telemetry.LogCollector.EXCLUDE_TELEMETRY; import datadog.trace.api.iast.util.PropagationUtils; +import datadog.trace.util.AdaptiveLatch; import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; @@ -10,6 +11,7 @@ import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.function.Supplier; +import javax.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -146,12 +148,54 @@ public static URI safeParse(final String unparsed) { * {@code ]} in the path, a {@code %} that is not followed by two hex digits, and a second {@code * #}. Well-formed URLs are converted exactly as {@code url.toURI()} would, with no extra * allocation. Other problems, such as an invalid scheme, still throw {@link URISyntaxException}. + * + *

    Bad URLs tend to come from the same application code again and again, so the repair runs + * only while they are arriving (see {@link ToURI}); otherwise this is just {@code url.toURI()}. */ public static URI toURI(final URL url) throws URISyntaxException { - return new URI(escapeIllegalURIChars(url.toString())); + final URI uri = TO_URI.tryApply(url); + // null only when the repair could not help: let URI report why + return uri != null ? uri : url.toURI(); + } + + private static final ToURI TO_URI = new ToURI(); + + /** + * Chooses between {@code url.toURI()} and the repairing conversion: strict while URLs are + * well-formed, so they skip the scan; repairing once a bad one arrives, until {@link + * #CLOSE_AFTER} well-formed ones in a row. + */ + static final class ToURI extends AdaptiveLatch { + /** + * The rent-or-buy break-even (see {@link AdaptiveLatch}): a thrown and caught {@link + * URISyntaxException} under an HTTP client's stack, against one extra scan of a well-formed + * URL. An estimate, not yet measured. + */ + static final int CLOSE_AFTER = 64; + + ToURI() { + super(URISyntaxException.class, CLOSE_AFTER); + } + + @Override + protected URI apply(final URL url) throws URISyntaxException { + return url.toURI(); + } + + @Override + protected URI applySafely(final URL url) { + final String s = url.toString(); + final String escaped = escapeIllegalURIChars(s); + try { + return escaped == null ? new URI(s) : repaired(new URI(escaped)); + } catch (URISyntaxException e) { + return reject(url); + } + } } - /** Percent-encodes the characters {@link URI} rejects; returns the same string if none. */ + /** Percent-encodes the characters {@link URI} rejects; returns {@code null} if there are none. */ + @Nullable private static String escapeIllegalURIChars(final String s) { final int length = s.length(); final int pathStart = pathStart(s); @@ -188,7 +232,7 @@ private static String escapeIllegalURIChars(final String s) { escaped.append(c); } } - return escaped == null ? s : escaped.toString(); + return escaped == null ? null : escaped.toString(); } /** Index where the path starts: after the authority of {@code scheme://authority}, or 0. */ diff --git a/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/URIUtilsToURITest.java b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/URIUtilsToURITest.java index 1234db3e2ca..3dc16d6e58d 100644 --- a/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/URIUtilsToURITest.java +++ b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/URIUtilsToURITest.java @@ -1,7 +1,10 @@ package datadog.trace.bootstrap.instrumentation.api; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.net.URISyntaxException; import java.net.URL; @@ -66,4 +69,47 @@ void stillThrowsForProblemsItDoesNotRepair() throws Exception { assertThrows(URISyntaxException.class, () -> URIUtils.toURI(url)); } + + @Test + void wellFormedUrlsTakeTheStrictPath() throws Exception { + URIUtils.ToURI latch = new URIUtils.ToURI(); + + assertEquals( + "http://example.com/a", latch.tryApply(new URL("http://example.com/a")).toString()); + assertFalse(latch.isEngaged()); + } + + @Test + void aBadUrlEngagesTheRepairAndRepairsKeepItEngaged() throws Exception { + URIUtils.ToURI latch = new URIUtils.ToURI(); + + assertEquals( + "http://example.com/a%20b", latch.tryApply(new URL("http://example.com/a b")).toString()); + assertTrue(latch.isEngaged()); + + for (int i = 0; i < URIUtils.ToURI.CLOSE_AFTER; i++) { + latch.tryApply(new URL("http://example.com/c d")); + } + assertTrue(latch.isEngaged(), "repaired URLs restart the count"); + } + + @Test + void wellFormedUrlsDisengageTheRepair() throws Exception { + URIUtils.ToURI latch = new URIUtils.ToURI(); + latch.tryApply(new URL("http://example.com/a b")); + + for (int i = 0; i < URIUtils.ToURI.CLOSE_AFTER; i++) { + assertEquals( + "http://example.com/ok", latch.tryApply(new URL("http://example.com/ok")).toString()); + } + assertFalse(latch.isEngaged()); + } + + @Test + void anUnrepairableUrlYieldsNullFromTheLatch() throws Exception { + URIUtils.ToURI latch = new URIUtils.ToURI(); + + assertNull(latch.tryApply(new URL("http", "[::1", -1, "/x"))); + assertTrue(latch.isEngaged()); + } } From df93969e89bf3debc5d9bf75273e1f92c354d650 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 7 Oct 2026 16:07:05 -0400 Subject: [PATCH 5/5] Add URIUtilsToURIBenchmark Compares url.toURI() (master), the latched URIUtils.toURI, the latch's repair path on every call, and reading the URL's fields directly with no URI, each on well-formed and bad URLs at stack depth 0 and 50. Co-Authored-By: Claude Opus 5.5 --- .../api/URIUtilsToURIBenchmark.java | 168 ++++++++++++++++++ 1 file changed, 168 insertions(+) create mode 100644 internal-api/src/jmh/java/datadog/trace/bootstrap/instrumentation/api/URIUtilsToURIBenchmark.java diff --git a/internal-api/src/jmh/java/datadog/trace/bootstrap/instrumentation/api/URIUtilsToURIBenchmark.java b/internal-api/src/jmh/java/datadog/trace/bootstrap/instrumentation/api/URIUtilsToURIBenchmark.java new file mode 100644 index 00000000000..7442ca02cd2 --- /dev/null +++ b/internal-api/src/jmh/java/datadog/trace/bootstrap/instrumentation/api/URIUtilsToURIBenchmark.java @@ -0,0 +1,168 @@ +package datadog.trace.bootstrap.instrumentation.api; + +import java.net.MalformedURLException; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; +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.Level; +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.Threads; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +/** + * What an HTTP client decorator pays to get the URL parts it tags from a {@link URL}, comparing + * conversions to {@link URI} against reading the {@code URL} directly. + * + *

    Every arm extracts the same values {@code HttpClientDecorator.onRequest} reads: scheme, host, + * port, and the decoded path, query and fragment. {@code URI}'s getters decode percent-escapes and + * {@code URL}'s do not, so the {@code URL} arm decodes them itself. + * + *

      + *
    • {@link #strict}: {@code url.toURI()}, as on master; a bad URL throws and is caught. + *
    • {@link #latched}: {@link URIUtils#toURI} through its shared {@link URIUtils.ToURI} latch, + * as in production. Well-formed input keeps it disengaged; bad input keeps it on the repair + * path. + *
    • {@link #repairPath}: the latch's repairing conversion on every call, which is what a + * well-formed URL costs while the latch is engaged. + *
    • {@link #urlFields}: no {@code URI} at all; the parts come from the {@code URL}, the + * structural alternative of a client-side {@code URIDataAdapter}. + *
    + * + *

    {@code depth} runs each call under that many extra frames, because a throw's cost grows with + * the stack it captures and a real HTTP client call is deep. Only {@link #strict} on bad input + * throws; the frames cost the other arms the same small constant. + * + *

    The latch is shared across all threads, as the production {@code static final} is, so its + * plain counter is written concurrently on the repair path. Run with {@code -prof gc}, and read the + * allocation modes across forks rather than their mean. + */ +@State(Scope.Benchmark) +@BenchmarkMode(Mode.Throughput) +@OutputTimeUnit(TimeUnit.SECONDS) +@Warmup(iterations = 5, time = 2) +@Measurement(iterations = 5, time = 2) +@Fork(3) +@Threads(8) +public class URIUtilsToURIBenchmark { + + @Param({"0", "50"}) + int depth; + + /** Shared like the production latch, which is a {@code static final}. */ + URIUtils.ToURI latch; + + /** Per thread, so each thread reads its own {@code URL}. */ + @State(Scope.Thread) + public static class Url { + @Param({"wellFormed", "bad"}) + String input; + + URL url; + + @Setup(Level.Trial) + @SuppressWarnings("deprecation") // new URL(String) is what applications use to build these + public void setup() throws MalformedURLException { + // same length and shape; the bad one has the unencoded space URI rejects and URL accepts + url = + new URL( + "wellFormed".equals(input) + ? "http://example.com/api/v1/search?q=hello%20world&page=2#top" + : "http://example.com/api/v1/search?q=hello world&page=2#top"); + } + } + + @Setup(Level.Trial) + public void setup() { + latch = new URIUtils.ToURI(); + } + + @Benchmark + public void strict(Url state, Blackhole bh) { + consume(bh, strict(state.url, depth)); + } + + @Benchmark + public void latched(Url state, Blackhole bh) { + consume(bh, latched(latch, state.url, depth)); + } + + @Benchmark + public void repairPath(Url state, Blackhole bh) { + consume(bh, repairPath(latch, state.url, depth)); + } + + @Benchmark + public void urlFields(Url state, Blackhole bh) { + urlFields(bh, state.url, depth); + } + + private static URI strict(URL url, int depth) { + if (depth > 0) { + return strict(url, depth - 1); + } + try { + return url.toURI(); + } catch (URISyntaxException e) { + return null; // HttpClientDecorator logs at debug and tags nothing + } + } + + private static URI latched(URIUtils.ToURI latch, URL url, int depth) { + if (depth > 0) { + return latched(latch, url, depth - 1); + } + final URI uri = latch.tryApply(url); + if (uri != null) { + return uri; + } + try { + return url.toURI(); // what URIUtils.toURI does for an unrepairable URL + } catch (URISyntaxException e) { + return null; + } + } + + private static URI repairPath(URIUtils.ToURI latch, URL url, int depth) { + if (depth > 0) { + return repairPath(latch, url, depth - 1); + } + return latch.applySafely(url); + } + + private static void urlFields(Blackhole bh, URL url, int depth) { + if (depth > 0) { + urlFields(bh, url, depth - 1); + return; + } + bh.consume(url.getProtocol()); + bh.consume(url.getHost()); + bh.consume(url.getPort()); + bh.consume(URIUtils.decode(url.getPath())); + bh.consume(URIUtils.decode(url.getQuery())); + bh.consume(URIUtils.decode(url.getRef())); + } + + private static void consume(Blackhole bh, URI uri) { + if (uri == null) { + bh.consume((Object) null); + return; + } + bh.consume(uri.getScheme()); + bh.consume(uri.getHost()); + bh.consume(uri.getPort()); + bh.consume(uri.getPath()); + bh.consume(uri.getQuery()); + bh.consume(uri.getFragment()); + } +}