From 252857a3b8c1bea64183ca1ee19b2c0310587d1c Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 7 Oct 2026 17:25:25 -0400 Subject: [PATCH 1/6] Fix Oracle and jTDS JDBC URL parsing failures Oracle: a second ':' after the port (EZConnect :server_type, IPv6 literals, EZConnect Plus ?params) or a protocol prefix other than ldap:// (tcp://, tcps://) was read as host:port:sid, so a non-numeric segment was parsed as the port and threw NumberFormatException. The service form now strips the optional :server_type and /instance_name suffixes, any :// prefix is skipped, ?params are dropped, and port parsing no longer throws. jTDS: the port and database separators were searched across the whole URL remainder from an offset derived from the type name, so a ':' or '/' in a property value threw StringIndexOutOfBoundsException and short host names lost their port. The address is now split at ';', then '/', then ':'. In both cases the exception was caught in parse(), dropping host, port and instance from DB spans. Co-Authored-By: Claude Opus 5.5 --- .../jdbc/JDBCConnectionUrlParser.java | 107 +++++++++++------- .../jdbc/JDBCConnectionUrlParserJtdsTest.java | 36 ++++++ .../JDBCConnectionUrlParserOracleTest.java | 39 +++++++ 3 files changed, 144 insertions(+), 38 deletions(-) create mode 100644 dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParserJtdsTest.java create mode 100644 dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParserOracleTest.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java index 617b8d707ce..76a14b02790 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java @@ -390,31 +390,37 @@ DBInfo.Builder doParse(String jdbcUrl, final DBInfo.Builder builder) { ORACLE_CONNECT_INFO() { @Override - DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) { + DBInfo.Builder doParse(final String connectInfo, final DBInfo.Builder builder) { final String host; final Integer port; final String instance; - final int hostEnd = jdbcUrl.indexOf(':'); - final int instanceLoc = jdbcUrl.indexOf('/'); + // EZConnect Plus parameters (?key=value) are not part of the address + final int queryLoc = connectInfo.indexOf('?'); + final String jdbcUrl = queryLoc >= 0 ? connectInfo.substring(0, queryLoc) : connectInfo; + + // skip a bracketed IPv6 literal so its colons are not taken as the port separator + final int hostSearchStart = jdbcUrl.startsWith("[") ? Math.max(jdbcUrl.indexOf(']'), 0) : 0; + final int instanceLoc = jdbcUrl.indexOf('/', hostSearchStart); + final int colonLoc = jdbcUrl.indexOf(':', hostSearchStart); + // a ':' after the service name separates the server type, not the port + final int hostEnd = instanceLoc >= 0 && colonLoc > instanceLoc ? -1 : colonLoc; if (hostEnd > 0) { host = jdbcUrl.substring(0, hostEnd); final int afterHostEnd = jdbcUrl.indexOf(':', hostEnd + 1); - if (afterHostEnd > 0) { - port = Integer.parseInt(jdbcUrl.substring(hostEnd + 1, afterHostEnd)); + if (afterHostEnd > 0 && (instanceLoc < 0 || afterHostEnd < instanceLoc)) { + // host:port:sid + port = parsePort(jdbcUrl.substring(hostEnd + 1, afterHostEnd)); instance = jdbcUrl.substring(afterHostEnd + 1); } else { if (instanceLoc > 0) { - instance = jdbcUrl.substring(instanceLoc + 1); - port = Integer.parseInt(jdbcUrl.substring(hostEnd + 1, instanceLoc)); + // host:port/service[:server_type][/instance_name] + instance = serviceName(jdbcUrl.substring(instanceLoc + 1)); + port = parsePort(jdbcUrl.substring(hostEnd + 1, instanceLoc)); } else { final String portOrInstance = jdbcUrl.substring(hostEnd + 1); - Integer parsedPort = null; - try { - parsedPort = Integer.parseInt(portOrInstance); - } catch (final NumberFormatException ignored) { - } + final Integer parsedPort = parsePort(portOrInstance); if (parsedPort == null) { port = null; instance = portOrInstance; @@ -428,7 +434,7 @@ DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) { if (instanceLoc > 0) { host = jdbcUrl.substring(0, instanceLoc); port = null; - instance = jdbcUrl.substring(instanceLoc + 1); + instance = serviceName(jdbcUrl.substring(instanceLoc + 1)); } else { if (jdbcUrl.isEmpty()) { return builder; @@ -447,6 +453,28 @@ DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) { } return builder.instance(instance); } + + private Integer parsePort(final String port) { + try { + return Integer.parseInt(port); + } catch (final NumberFormatException ignored) { + return null; + } + } + + /** Drops the optional {@code :server_type} and {@code /instance_name} EZConnect suffixes. */ + private String serviceName(final String service) { + int end = service.length(); + final int serverTypeLoc = service.indexOf(':'); + if (serverTypeLoc >= 0) { + end = serverTypeLoc; + } + final int instanceNameLoc = service.indexOf('/'); + if (instanceNameLoc >= 0 && instanceNameLoc < end) { + end = instanceNameLoc; + } + return service.substring(0, end); + } }, ORACLE_AT() { @@ -469,10 +497,12 @@ DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) { } final int hostStart; + final int protocolEnd = connectInfo.indexOf("://"); if (connectInfo.startsWith("//")) { hostStart = "//".length(); - } else if (connectInfo.startsWith("ldap://")) { - hostStart = "ldap://".length(); + } else if (protocolEnd > 0 && protocolEnd == connectInfo.indexOf(':')) { + // protocol prefix, e.g. ldap://, tcp://, tcps:// + hostStart = protocolEnd + "://".length(); } else { hostStart = 0; } @@ -726,6 +756,10 @@ DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) { final int protoLoc = jdbcUrl.indexOf("://"); final int typeEndLoc = dbInfo.getType().length(); + if (protoLoc <= typeEndLoc) { + // no "jtds:://" prefix to parse + return builder; + } final String subtype = jdbcUrl.substring(typeEndLoc + 1, protoLoc); builder.subtype(subtype); @@ -740,41 +774,38 @@ DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) { } } + // [:][/][;=[;...]] final String details = jdbcUrl.substring(protoLoc + "://".length()); - final int hostEndLoc; - final int portLoc = details.indexOf(':', typeEndLoc + 1); - final int dbLoc = details.indexOf('/', typeEndLoc); - final int paramLoc = details.indexOf(';', dbLoc); - - if (paramLoc > 0) { + final int paramLoc = details.indexOf(';'); + final String address; + if (paramLoc >= 0) { populateStandardProperties(builder, splitQuery(details.substring(paramLoc + 1), ';')); - if (dbLoc > 0) { - builder.db(details.substring(dbLoc + 1, paramLoc)); - } + address = details.substring(0, paramLoc); } else { - if (dbLoc > 0) { - builder.db(details.substring(dbLoc + 1)); - } + address = details; } - if (portLoc > 0) { - hostEndLoc = portLoc; - final int portEndLoc = dbLoc > 0 ? dbLoc : (paramLoc > 0 ? paramLoc : details.length()); + final int dbLoc = address.indexOf('/'); + final String hostAndPort; + if (dbLoc >= 0) { + builder.db(address.substring(dbLoc + 1)); + hostAndPort = address.substring(0, dbLoc); + } else { + hostAndPort = address; + } + + final int portLoc = hostAndPort.indexOf(':'); + if (portLoc >= 0) { try { - builder.port(Integer.parseInt(details.substring(portLoc + 1, portEndLoc))); + builder.port(Integer.parseInt(hostAndPort.substring(portLoc + 1))); } catch (final NumberFormatException ignored) { } - } else if (dbLoc > 0) { - hostEndLoc = dbLoc; - } else if (paramLoc > 0) { - hostEndLoc = paramLoc; + builder.host(hostAndPort.substring(0, portLoc)); } else { - hostEndLoc = details.length(); + builder.host(hostAndPort); } - builder.host(details.substring(0, hostEndLoc)); - return builder; } }, diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParserJtdsTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParserJtdsTest.java new file mode 100644 index 00000000000..2c652abf324 --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParserJtdsTest.java @@ -0,0 +1,36 @@ +package datadog.trace.bootstrap.instrumentation.jdbc; + +import static datadog.trace.bootstrap.instrumentation.jdbc.JDBCConnectionUrlParser.extractDBInfo; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.tabletest.junit.TableTest; + +/** + * Tests for jTDS URLs ({@code jdbc:jtds:://[:][/][;]}). + * + *

The port and database separators used to be searched for across the whole remainder of the + * URL, starting at an offset unrelated to it, so a ':' or '/' in a property value or a short host + * name produced a {@code StringIndexOutOfBoundsException} or a wrong host. + */ +class JDBCConnectionUrlParserJtdsTest { + + @TableTest({ + "scenario | url | subtype | host | port | db ", + "host port and db | jdbc:jtds:sqlserver://dbhost:1500/mydb | sqlserver | dbhost | 1500 | mydb", + "colon in property after db | jdbc:jtds:sqlserver://dbhost:1500/mydb;appname=a:b | sqlserver | dbhost | 1500 | mydb", + "colon in property, no port | jdbc:jtds:sqlserver://dbhost/mydb;password=p:w | sqlserver | dbhost | 1433 | mydb", + "colon in property, no db | jdbc:jtds:sqlserver://dbhost;password=p:w | sqlserver | dbhost | 1433 | ", + "slash in property, no db | jdbc:jtds:sqlserver://dbhost;password=p/w | sqlserver | dbhost | 1433 | ", + "short host with port | jdbc:jtds:sqlserver://db:1500/mydb | sqlserver | db | 1500 | mydb", + "sybase with port | jdbc:jtds:sybase://dbhost:7200/mydb | sybase | dbhost | 7200 | mydb", + "missing protocol separator | jdbc:jtds:sqlserver:dbhost | | | | " + }) + void parsesJtdsUrls(String url, String subtype, String host, Integer port, String db) { + DBInfo info = extractDBInfo(url, null); + assertEquals("jtds", info.getType()); + assertEquals(subtype, info.getSubtype()); + assertEquals(host, info.getHost()); + assertEquals(port, info.getPort()); + assertEquals(db, info.getDb()); + } +} diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParserOracleTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParserOracleTest.java new file mode 100644 index 00000000000..2f55579d3f6 --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParserOracleTest.java @@ -0,0 +1,39 @@ +package datadog.trace.bootstrap.instrumentation.jdbc; + +import static datadog.trace.bootstrap.instrumentation.jdbc.JDBCConnectionUrlParser.extractDBInfo; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.tabletest.junit.TableTest; + +/** + * Tests for Oracle {@code @} connect strings that previously threw {@code NumberFormatException}. + * + *

A second ':' after the port (EZConnect {@code :server_type}, IPv6 literals, {@code ?params}) + * or a protocol prefix other than {@code ldap://} was taken as the {@code host:port:sid} form, so a + * non-numeric segment was parsed as the port and the whole URL fell back to defaults. + */ +class JDBCConnectionUrlParserOracleTest { + + @TableTest({ + "scenario | url | host | port | instance", + "SID form | jdbc:oracle:thin:@orcl.host:55:orclsn | orcl.host | 55 | orclsn ", + "service form | jdbc:oracle:thin:@//orcl.host:55/orclsn | orcl.host | 55 | orclsn ", + "service with server type | jdbc:oracle:thin:@//orcl.host:55/orclsn:dedicated | orcl.host | 55 | orclsn ", + "service with type and instance | jdbc:oracle:thin:@//orcl.host:55/orclsn:pooled/inst1 | orcl.host | 55 | orclsn ", + "service with instance name | jdbc:oracle:thin:@//orcl.host:55/orclsn/inst1 | orcl.host | 55 | orclsn ", + "service without port | jdbc:oracle:thin:@//orcl.host/orclsn:dedicated | orcl.host | 1521 | orclsn ", + "tcps protocol | jdbc:oracle:thin:@tcps://orcl.host:2484/orclsn | orcl.host | 2484 | orclsn ", + "tcp protocol | jdbc:oracle:thin:@tcp://orcl.host:55/orclsn | orcl.host | 55 | orclsn ", + "EZConnect Plus params | jdbc:oracle:thin:@tcps://orcl.host:2484/orclsn?wallet_location=c:/w | orcl.host | 2484 | orclsn ", + "IPv6 literal | jdbc:oracle:thin:@//[::1]:55/orclsn | '[::1]' | 55 | orclsn ", + "non-numeric port | jdbc:oracle:thin:@orcl.host:abc:orclsn | orcl.host | 1521 | orclsn " + }) + void parsesOracleConnectStrings(String url, String host, Integer port, String instance) { + DBInfo info = extractDBInfo(url, null); + assertEquals("oracle", info.getType()); + assertEquals("thin", info.getSubtype()); + assertEquals(host, info.getHost()); + assertEquals(port, info.getPort()); + assertEquals(instance, info.getInstance()); + } +} From 6c8ab813d5c0b28e287fdffd2512079715036c5b Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 7 Oct 2026 17:42:21 -0400 Subject: [PATCH 2/6] Share parsePort between the Oracle and jTDS URL parsers Co-Authored-By: Claude Opus 5.5 --- .../jdbc/JDBCConnectionUrlParser.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java index 76a14b02790..b3ab2e0c968 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java @@ -454,14 +454,6 @@ DBInfo.Builder doParse(final String connectInfo, final DBInfo.Builder builder) { return builder.instance(instance); } - private Integer parsePort(final String port) { - try { - return Integer.parseInt(port); - } catch (final NumberFormatException ignored) { - return null; - } - } - /** Drops the optional {@code :server_type} and {@code /instance_name} EZConnect suffixes. */ private String serviceName(final String service) { int end = service.length(); @@ -797,9 +789,9 @@ DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) { final int portLoc = hostAndPort.indexOf(':'); if (portLoc >= 0) { - try { - builder.port(Integer.parseInt(hostAndPort.substring(portLoc + 1))); - } catch (final NumberFormatException ignored) { + final Integer port = parsePort(hostAndPort.substring(portLoc + 1)); + if (port != null) { + builder.port(port); } builder.host(hostAndPort.substring(0, portLoc)); } else { @@ -931,6 +923,14 @@ public static DBInfo parse(String connectionUrl, final Properties props) { // Source: https://stackoverflow.com/a/13592567 @SuppressForbidden + private static Integer parsePort(final String port) { + try { + return Integer.parseInt(port); + } catch (final NumberFormatException ignored) { + return null; + } + } + private static Map splitQuery(final String query, final char separator) { if (query == null || query.isEmpty()) { return Collections.emptyMap(); From 834714508f4c6852f42940f1f6ed2ab5afd932e1 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 7 Oct 2026 17:47:02 -0400 Subject: [PATCH 3/6] Parse JDBC URL ports without exceptions Add Numbers.parseNonNegativeInt(CharSequence[, start, end]), which returns -1 for empty, non-digit, signed or overflowing input instead of throwing, and parses a range in place without a substring. Route every port parse in JDBCConnectionUrlParser through it. This removes the catch-and-ignore NumberFormatException blocks, and the sites that let the exception escape to parse()'s catch-all (MODIFIED_URL_LIKE, DERBY, the regex-matched ORACLE_AT_DESCRIPTION / MARIA_ADDRESS ports) now keep host and instance when the port is malformed instead of discarding the whole URL. Co-Authored-By: Claude Opus 5.5 --- .../jdbc/JDBCConnectionUrlParser.java | 67 +++++++++---------- .../main/java/datadog/trace/util/Numbers.java | 32 +++++++++ .../java/datadog/trace/util/NumbersTest.java | 41 ++++++++++++ 3 files changed, 105 insertions(+), 35 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java index b3ab2e0c968..51c18e94b57 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java @@ -1,6 +1,7 @@ package datadog.trace.bootstrap.instrumentation.jdbc; import static datadog.trace.bootstrap.instrumentation.jdbc.DBInfo.DEFAULT; +import static datadog.trace.util.Numbers.parseNonNegativeInt; import static java.lang.Math.max; import datadog.trace.api.Pair; @@ -140,7 +141,7 @@ DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) { final int portLoc = serverName.indexOf(':'); if (portLoc > 1) { - port = Integer.parseInt(serverName.substring(portLoc + 1)); + port = parsePort(serverName, portLoc + 1, serverName.length()); serverName = serverName.substring(0, portLoc); } @@ -221,10 +222,7 @@ DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) { if (portLoc > 0) { hostEndLoc = portLoc; - try { - builder.port(Integer.parseInt(jdbcUrl.substring(portLoc + 1, dbLoc))); - } catch (final NumberFormatException ignored) { - } + setPort(builder, parsePort(jdbcUrl, portLoc + 1, dbLoc)); } else { hostEndLoc = dbLoc; } @@ -268,10 +266,7 @@ DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) { if (portLoc > 0) { hostEndLoc = portLoc; final int portEndLoc = clusterSepLoc > 0 ? clusterSepLoc : dbLoc; - try { - builder.port(Integer.parseInt(jdbcUrl.substring(portLoc + 1, portEndLoc))); - } catch (final NumberFormatException ignored) { - } + setPort(builder, parsePort(jdbcUrl, portLoc + 1, portEndLoc)); } else { hostEndLoc = clusterSepLoc > 0 ? clusterSepLoc : dbLoc; } @@ -303,7 +298,7 @@ DBInfo.Builder doParse(String jdbcUrl, final DBInfo.Builder builder) { final Matcher portMatcher = PORT_REGEX.matcher(jdbcUrl); if (portMatcher.find()) { - builder.port(Integer.parseInt(portMatcher.group(1))); + setPort(builder, parsePort(portMatcher.group(1))); } final Matcher userMatcher = USER_REGEX.matcher(jdbcUrl); @@ -411,13 +406,13 @@ DBInfo.Builder doParse(final String connectInfo, final DBInfo.Builder builder) { final int afterHostEnd = jdbcUrl.indexOf(':', hostEnd + 1); if (afterHostEnd > 0 && (instanceLoc < 0 || afterHostEnd < instanceLoc)) { // host:port:sid - port = parsePort(jdbcUrl.substring(hostEnd + 1, afterHostEnd)); + port = parsePort(jdbcUrl, hostEnd + 1, afterHostEnd); instance = jdbcUrl.substring(afterHostEnd + 1); } else { if (instanceLoc > 0) { // host:port/service[:server_type][/instance_name] instance = serviceName(jdbcUrl.substring(instanceLoc + 1)); - port = parsePort(jdbcUrl.substring(hostEnd + 1, instanceLoc)); + port = parsePort(jdbcUrl, hostEnd + 1, instanceLoc); } else { final String portOrInstance = jdbcUrl.substring(hostEnd + 1); final Integer parsedPort = parsePort(portOrInstance); @@ -533,7 +528,7 @@ DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) { final Matcher portMatcher = PORT_REGEX.matcher(urlPart2); if (portMatcher.find()) { - builder.port(Integer.parseInt(portMatcher.group(1))); + setPort(builder, parsePort(portMatcher.group(1))); } final Matcher instanceMatcher = INSTANCE_REGEX.matcher(urlPart2); @@ -719,7 +714,7 @@ DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) { final int portLoc = url.indexOf(':'); if (portLoc > 0) { host = url.substring(0, portLoc); - builder.port(Integer.parseInt(url.substring(portLoc + 1))); + setPort(builder, parsePort(url, portLoc + 1, url.length())); } else { host = url; } @@ -789,10 +784,7 @@ DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) { final int portLoc = hostAndPort.indexOf(':'); if (portLoc >= 0) { - final Integer port = parsePort(hostAndPort.substring(portLoc + 1)); - if (port != null) { - builder.port(port); - } + setPort(builder, parsePort(hostAndPort, portLoc + 1, hostAndPort.length())); builder.host(hostAndPort.substring(0, portLoc)); } else { builder.host(hostAndPort); @@ -923,11 +915,26 @@ public static DBInfo parse(String connectionUrl, final Properties props) { // Source: https://stackoverflow.com/a/13592567 @SuppressForbidden - private static Integer parsePort(final String port) { - try { - return Integer.parseInt(port); - } catch (final NumberFormatException ignored) { - return null; + /** + * @return the port, or {@code null} if {@code s[start, end)} is not a valid port number + */ + private static Integer parsePort(final CharSequence s, final int start, final int end) { + final int port = parseNonNegativeInt(s, start, end); + return port >= 0 ? port : null; + } + + /** + * @return the port, or {@code null} if {@code s} is null or not a valid port number + */ + private static Integer parsePort(final String s) { + final int port = parseNonNegativeInt(s); + return port >= 0 ? port : null; + } + + /** Sets the port only when one was parsed, keeping any default already on the builder. */ + private static void setPort(final DBInfo.Builder builder, final Integer port) { + if (port != null) { + builder.port(port); } } @@ -987,12 +994,7 @@ private static void populateStandardProperties( } if (props.containsKey("portnumber")) { - final String portNumber = (String) props.get("portnumber"); - try { - builder.port(Integer.parseInt(portNumber)); - } catch (final NumberFormatException e) { - ExceptionLogger.LOGGER.debug("Error parsing portnumber property: {}", portNumber, e); - } + setPort(builder, parsePort((String) props.get("portnumber"))); } if (props.containsKey("servicename")) { // this property is used to specify the db to use for Sybase connection strings @@ -1000,12 +1002,7 @@ private static void populateStandardProperties( } if (props.containsKey("portNumber")) { - final String portNumber = (String) props.get("portNumber"); - try { - builder.port(Integer.parseInt(portNumber)); - } catch (final NumberFormatException e) { - ExceptionLogger.LOGGER.debug("Error parsing portNumber property: {}", portNumber, e); - } + setPort(builder, parsePort((String) props.get("portNumber"))); } } } diff --git a/internal-api/src/main/java/datadog/trace/util/Numbers.java b/internal-api/src/main/java/datadog/trace/util/Numbers.java index 284aa4b733a..a4ecf86f4ae 100644 --- a/internal-api/src/main/java/datadog/trace/util/Numbers.java +++ b/internal-api/src/main/java/datadog/trace/util/Numbers.java @@ -14,6 +14,38 @@ private Numbers() { // Utility class - prevent instantiation } + /** + * Parses a non-negative decimal {@code int} from the whole sequence without throwing. + * + * @return the parsed value, or {@code -1} if {@code s} is null or empty, contains anything other + * than ASCII digits (including a sign), or overflows {@code int} + */ + public static int parseNonNegativeInt(@Nullable CharSequence s) { + return s == null ? -1 : parseNonNegativeInt(s, 0, s.length()); + } + + /** + * Parses a non-negative decimal {@code int} from {@code s[start, end)} without throwing or + * allocating. + * + * @return the parsed value, or {@code -1} if the range is empty or out of bounds, contains + * anything other than ASCII digits (including a sign), or overflows {@code int} + */ + public static int parseNonNegativeInt(@Nullable CharSequence s, int start, int end) { + if (s == null || start < 0 || end > s.length() || start >= end) { + return -1; + } + int result = 0; + for (int i = start; i < end; i++) { + int digit = s.charAt(i) - '0'; + if (digit < 0 || digit > 9 || result > (Integer.MAX_VALUE - digit) / 10) { + return -1; + } + result = result * 10 + digit; + } + return result; + } + /** * Attempts to parse a string value as a number with fast-path validation to avoid throwing * NumberFormatException for invalid inputs. diff --git a/internal-api/src/test/java/datadog/trace/util/NumbersTest.java b/internal-api/src/test/java/datadog/trace/util/NumbersTest.java index 816f7e65020..857311d05ff 100644 --- a/internal-api/src/test/java/datadog/trace/util/NumbersTest.java +++ b/internal-api/src/test/java/datadog/trace/util/NumbersTest.java @@ -1,5 +1,6 @@ package datadog.trace.util; +import static datadog.trace.util.Numbers.parseNonNegativeInt; import static datadog.trace.util.Numbers.parseNumber; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -144,4 +145,44 @@ void parseNumberHandlesEdgeCasesForTypeChecking() { assertInstanceOf(Double.class, doubleResult); assertInstanceOf(Double.class, sciResult); } + + @TableTest({ + "scenario | input | expected ", + "zero | 0 | 0 ", + "port | 1521 | 1521 ", + "leading zeros | 007 | 7 ", + "int max | 2147483647 | 2147483647", + "int max plus one | 2147483648 | -1 ", + "far overflow | 99999999999 | -1 ", + "empty | '' | -1 ", + "minus sign | -1 | -1 ", + "plus sign | +1 | -1 ", + "letters | abc | -1 ", + "trailing garbage | 12a | -1 ", + "whitespace | ' 1' | -1 ", + "non-ASCII digit | '\u0661' | -1 " + }) + void parseNonNegativeIntWholeSequence(String input, int expected) { + assertEquals(expected, parseNonNegativeInt(input)); + } + + @TableTest({ + "scenario | input | start | end | expected", + "middle of string | host:1521/db | 5 | 9 | 1521 ", + "to end | host:1521 | 5 | 9 | 1521 ", + "empty range | host:/db | 5 | 5 | -1 ", + "inverted range | host:1521 | 6 | 5 | -1 ", + "negative start | 1521 | -1 | 4 | -1 ", + "end past length | 1521 | 0 | 5 | -1 ", + "range spans garbage | host:1521/db | 4 | 9 | -1 " + }) + void parseNonNegativeIntRange(String input, int start, int end, int expected) { + assertEquals(expected, parseNonNegativeInt(input, start, end)); + } + + @Test + void parseNonNegativeIntNull() { + assertEquals(-1, parseNonNegativeInt(null)); + assertEquals(-1, parseNonNegativeInt(null, 0, 0)); + } } From 977b61ebd2faf51fe12ab3862458028cd27b907c Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 7 Oct 2026 17:51:18 -0400 Subject: [PATCH 4/6] Take (start, len) in Numbers.parseNonNegativeInt to match LongStringUtils Co-Authored-By: Claude Opus 5.5 --- .../jdbc/JDBCConnectionUrlParser.java | 2 +- .../main/java/datadog/trace/util/Numbers.java | 10 +++++----- .../java/datadog/trace/util/NumbersTest.java | 19 ++++++++++--------- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java index 51c18e94b57..807fc9cf261 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java @@ -919,7 +919,7 @@ public static DBInfo parse(String connectionUrl, final Properties props) { * @return the port, or {@code null} if {@code s[start, end)} is not a valid port number */ private static Integer parsePort(final CharSequence s, final int start, final int end) { - final int port = parseNonNegativeInt(s, start, end); + final int port = parseNonNegativeInt(s, start, end - start); return port >= 0 ? port : null; } diff --git a/internal-api/src/main/java/datadog/trace/util/Numbers.java b/internal-api/src/main/java/datadog/trace/util/Numbers.java index a4ecf86f4ae..62a20683679 100644 --- a/internal-api/src/main/java/datadog/trace/util/Numbers.java +++ b/internal-api/src/main/java/datadog/trace/util/Numbers.java @@ -25,18 +25,18 @@ public static int parseNonNegativeInt(@Nullable CharSequence s) { } /** - * Parses a non-negative decimal {@code int} from {@code s[start, end)} without throwing or - * allocating. + * Parses a non-negative decimal {@code int} from the {@code len} characters of {@code s} starting + * at {@code start}, without throwing or allocating. * * @return the parsed value, or {@code -1} if the range is empty or out of bounds, contains * anything other than ASCII digits (including a sign), or overflows {@code int} */ - public static int parseNonNegativeInt(@Nullable CharSequence s, int start, int end) { - if (s == null || start < 0 || end > s.length() || start >= end) { + public static int parseNonNegativeInt(@Nullable CharSequence s, int start, int len) { + if (s == null || start < 0 || len <= 0 || start > s.length() - len) { return -1; } int result = 0; - for (int i = start; i < end; i++) { + for (int i = start, end = start + len; i < end; i++) { int digit = s.charAt(i) - '0'; if (digit < 0 || digit > 9 || result > (Integer.MAX_VALUE - digit) / 10) { return -1; diff --git a/internal-api/src/test/java/datadog/trace/util/NumbersTest.java b/internal-api/src/test/java/datadog/trace/util/NumbersTest.java index 857311d05ff..72ee4b728d0 100644 --- a/internal-api/src/test/java/datadog/trace/util/NumbersTest.java +++ b/internal-api/src/test/java/datadog/trace/util/NumbersTest.java @@ -167,17 +167,18 @@ void parseNonNegativeIntWholeSequence(String input, int expected) { } @TableTest({ - "scenario | input | start | end | expected", - "middle of string | host:1521/db | 5 | 9 | 1521 ", - "to end | host:1521 | 5 | 9 | 1521 ", - "empty range | host:/db | 5 | 5 | -1 ", - "inverted range | host:1521 | 6 | 5 | -1 ", + "scenario | input | start | len | expected", + "middle of string | host:1521/db | 5 | 4 | 1521 ", + "to end | host:1521 | 5 | 4 | 1521 ", + "empty range | host:/db | 5 | 0 | -1 ", + "negative length | host:1521 | 6 | -1 | -1 ", "negative start | 1521 | -1 | 4 | -1 ", - "end past length | 1521 | 0 | 5 | -1 ", - "range spans garbage | host:1521/db | 4 | 9 | -1 " + "past end | 1521 | 1 | 4 | -1 ", + "start past length | 1521 | 5 | 1 | -1 ", + "range spans garbage | host:1521/db | 4 | 5 | -1 " }) - void parseNonNegativeIntRange(String input, int start, int end, int expected) { - assertEquals(expected, parseNonNegativeInt(input, start, end)); + void parseNonNegativeIntRange(String input, int start, int len, int expected) { + assertEquals(expected, parseNonNegativeInt(input, start, len)); } @Test From a97eccf86295ec1b54393d6220cce036ea5636ba Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 7 Oct 2026 18:23:47 -0400 Subject: [PATCH 5/6] Move parseNonNegativeInt into IntStringUtils Mirrors LongStringUtils naming. It lives in internal-api rather than next to LongStringUtils in dd-trace-api so the published API jar does not grow. Co-Authored-By: Claude Opus 5.5 --- .../jdbc/JDBCConnectionUrlParser.java | 2 +- .../datadog/trace/util/IntStringUtils.java | 41 +++++++++++++++ .../main/java/datadog/trace/util/Numbers.java | 32 ------------ .../trace/util/IntStringUtilsTest.java | 51 +++++++++++++++++++ .../java/datadog/trace/util/NumbersTest.java | 42 --------------- 5 files changed, 93 insertions(+), 75 deletions(-) create mode 100644 internal-api/src/main/java/datadog/trace/util/IntStringUtils.java create mode 100644 internal-api/src/test/java/datadog/trace/util/IntStringUtilsTest.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java index 807fc9cf261..cbebc626e16 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java @@ -1,7 +1,7 @@ package datadog.trace.bootstrap.instrumentation.jdbc; import static datadog.trace.bootstrap.instrumentation.jdbc.DBInfo.DEFAULT; -import static datadog.trace.util.Numbers.parseNonNegativeInt; +import static datadog.trace.util.IntStringUtils.parseNonNegativeInt; import static java.lang.Math.max; import datadog.trace.api.Pair; diff --git a/internal-api/src/main/java/datadog/trace/util/IntStringUtils.java b/internal-api/src/main/java/datadog/trace/util/IntStringUtils.java new file mode 100644 index 00000000000..9d18f830d20 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/util/IntStringUtils.java @@ -0,0 +1,41 @@ +package datadog.trace.util; + +import javax.annotation.Nullable; + +/** Exception-free {@code int} parsing, the {@code int} counterpart of {@code LongStringUtils}. */ +public final class IntStringUtils { + + private IntStringUtils() {} + + /** + * Parses a non-negative decimal {@code int} from the whole sequence without throwing. + * + * @return the parsed value, or {@code -1} if {@code s} is null or empty, contains anything other + * than ASCII digits (including a sign), or overflows {@code int} + */ + public static int parseNonNegativeInt(@Nullable CharSequence s) { + return s == null ? -1 : parseNonNegativeInt(s, 0, s.length()); + } + + /** + * Parses a non-negative decimal {@code int} from the {@code len} characters of {@code s} starting + * at {@code start}, without throwing or allocating. + * + * @return the parsed value, or {@code -1} if the range is empty or out of bounds, contains + * anything other than ASCII digits (including a sign), or overflows {@code int} + */ + public static int parseNonNegativeInt(@Nullable CharSequence s, int start, int len) { + if (s == null || start < 0 || len <= 0 || start > s.length() - len) { + return -1; + } + int result = 0; + for (int i = start, end = start + len; i < end; i++) { + int digit = s.charAt(i) - '0'; + if (digit < 0 || digit > 9 || result > (Integer.MAX_VALUE - digit) / 10) { + return -1; + } + result = result * 10 + digit; + } + return result; + } +} diff --git a/internal-api/src/main/java/datadog/trace/util/Numbers.java b/internal-api/src/main/java/datadog/trace/util/Numbers.java index 62a20683679..284aa4b733a 100644 --- a/internal-api/src/main/java/datadog/trace/util/Numbers.java +++ b/internal-api/src/main/java/datadog/trace/util/Numbers.java @@ -14,38 +14,6 @@ private Numbers() { // Utility class - prevent instantiation } - /** - * Parses a non-negative decimal {@code int} from the whole sequence without throwing. - * - * @return the parsed value, or {@code -1} if {@code s} is null or empty, contains anything other - * than ASCII digits (including a sign), or overflows {@code int} - */ - public static int parseNonNegativeInt(@Nullable CharSequence s) { - return s == null ? -1 : parseNonNegativeInt(s, 0, s.length()); - } - - /** - * Parses a non-negative decimal {@code int} from the {@code len} characters of {@code s} starting - * at {@code start}, without throwing or allocating. - * - * @return the parsed value, or {@code -1} if the range is empty or out of bounds, contains - * anything other than ASCII digits (including a sign), or overflows {@code int} - */ - public static int parseNonNegativeInt(@Nullable CharSequence s, int start, int len) { - if (s == null || start < 0 || len <= 0 || start > s.length() - len) { - return -1; - } - int result = 0; - for (int i = start, end = start + len; i < end; i++) { - int digit = s.charAt(i) - '0'; - if (digit < 0 || digit > 9 || result > (Integer.MAX_VALUE - digit) / 10) { - return -1; - } - result = result * 10 + digit; - } - return result; - } - /** * Attempts to parse a string value as a number with fast-path validation to avoid throwing * NumberFormatException for invalid inputs. diff --git a/internal-api/src/test/java/datadog/trace/util/IntStringUtilsTest.java b/internal-api/src/test/java/datadog/trace/util/IntStringUtilsTest.java new file mode 100644 index 00000000000..50e12c5e0b2 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/IntStringUtilsTest.java @@ -0,0 +1,51 @@ +package datadog.trace.util; + +import static datadog.trace.util.IntStringUtils.parseNonNegativeInt; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.Test; +import org.tabletest.junit.TableTest; + +class IntStringUtilsTest { + + @TableTest({ + "scenario | input | expected ", + "zero | 0 | 0 ", + "port | 1521 | 1521 ", + "leading zeros | 007 | 7 ", + "int max | 2147483647 | 2147483647", + "int max plus one | 2147483648 | -1 ", + "far overflow | 99999999999 | -1 ", + "empty | '' | -1 ", + "minus sign | -1 | -1 ", + "plus sign | +1 | -1 ", + "letters | abc | -1 ", + "trailing garbage | 12a | -1 ", + "whitespace | ' 1' | -1 ", + "non-ASCII digit | '\u0661' | -1 " + }) + void parseNonNegativeIntWholeSequence(String input, int expected) { + assertEquals(expected, parseNonNegativeInt(input)); + } + + @TableTest({ + "scenario | input | start | len | expected", + "middle of string | host:1521/db | 5 | 4 | 1521 ", + "to end | host:1521 | 5 | 4 | 1521 ", + "empty range | host:/db | 5 | 0 | -1 ", + "negative length | host:1521 | 6 | -1 | -1 ", + "negative start | 1521 | -1 | 4 | -1 ", + "past end | 1521 | 1 | 4 | -1 ", + "start past length | 1521 | 5 | 1 | -1 ", + "range spans garbage | host:1521/db | 4 | 5 | -1 " + }) + void parseNonNegativeIntRange(String input, int start, int len, int expected) { + assertEquals(expected, parseNonNegativeInt(input, start, len)); + } + + @Test + void parseNonNegativeIntNull() { + assertEquals(-1, parseNonNegativeInt(null)); + assertEquals(-1, parseNonNegativeInt(null, 0, 0)); + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/NumbersTest.java b/internal-api/src/test/java/datadog/trace/util/NumbersTest.java index 72ee4b728d0..816f7e65020 100644 --- a/internal-api/src/test/java/datadog/trace/util/NumbersTest.java +++ b/internal-api/src/test/java/datadog/trace/util/NumbersTest.java @@ -1,6 +1,5 @@ package datadog.trace.util; -import static datadog.trace.util.Numbers.parseNonNegativeInt; import static datadog.trace.util.Numbers.parseNumber; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -145,45 +144,4 @@ void parseNumberHandlesEdgeCasesForTypeChecking() { assertInstanceOf(Double.class, doubleResult); assertInstanceOf(Double.class, sciResult); } - - @TableTest({ - "scenario | input | expected ", - "zero | 0 | 0 ", - "port | 1521 | 1521 ", - "leading zeros | 007 | 7 ", - "int max | 2147483647 | 2147483647", - "int max plus one | 2147483648 | -1 ", - "far overflow | 99999999999 | -1 ", - "empty | '' | -1 ", - "minus sign | -1 | -1 ", - "plus sign | +1 | -1 ", - "letters | abc | -1 ", - "trailing garbage | 12a | -1 ", - "whitespace | ' 1' | -1 ", - "non-ASCII digit | '\u0661' | -1 " - }) - void parseNonNegativeIntWholeSequence(String input, int expected) { - assertEquals(expected, parseNonNegativeInt(input)); - } - - @TableTest({ - "scenario | input | start | len | expected", - "middle of string | host:1521/db | 5 | 4 | 1521 ", - "to end | host:1521 | 5 | 4 | 1521 ", - "empty range | host:/db | 5 | 0 | -1 ", - "negative length | host:1521 | 6 | -1 | -1 ", - "negative start | 1521 | -1 | 4 | -1 ", - "past end | 1521 | 1 | 4 | -1 ", - "start past length | 1521 | 5 | 1 | -1 ", - "range spans garbage | host:1521/db | 4 | 5 | -1 " - }) - void parseNonNegativeIntRange(String input, int start, int len, int expected) { - assertEquals(expected, parseNonNegativeInt(input, start, len)); - } - - @Test - void parseNonNegativeIntNull() { - assertEquals(-1, parseNonNegativeInt(null)); - assertEquals(-1, parseNonNegativeInt(null, 0, 0)); - } } From 18793b5180d1c39b12d24886217676103237fe90 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 8 Oct 2026 09:06:32 -0400 Subject: [PATCH 6/6] Parse the Play Host header port without exceptions RequestURIDataAdapter split the client-supplied Host header on its last ':' and ran Integer.parseInt on the rest, so a malformed port, or an IPv6 host without a port ("[::1]"), threw from the constructor. HttpServerDecorator catches that around url(request), which skipped http.url, http.hostname, the URL resource name and the AppSec request URI callback for the request. Add HostHeader (internal-api) to split host and port without throwing, treating ':' inside an IPv6 literal as part of the host, and use it in the Play 2.3, 2.4 and 2.6 adapters. A missing or malformed port is reported as 0, as a missing port already was. Co-Authored-By: Claude Opus 5.5 --- .../play23/RequestURIDataAdapter.java | 10 ++-- .../play24/RequestURIDataAdapter.java | 10 ++-- .../play26/RequestURIDataAdapter.java | 10 ++-- .../instrumentation/api/HostHeader.java | 46 +++++++++++++++++++ .../instrumentation/api/HostHeaderTest.java | 35 ++++++++++++++ 5 files changed, 102 insertions(+), 9 deletions(-) create mode 100644 internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/HostHeader.java create mode 100644 internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/HostHeaderTest.java diff --git a/dd-java-agent/instrumentation/play/play-2.3/src/main/java/datadog/trace/instrumentation/play23/RequestURIDataAdapter.java b/dd-java-agent/instrumentation/play/play-2.3/src/main/java/datadog/trace/instrumentation/play23/RequestURIDataAdapter.java index f91358d52f4..08d9e681e29 100644 --- a/dd-java-agent/instrumentation/play/play-2.3/src/main/java/datadog/trace/instrumentation/play23/RequestURIDataAdapter.java +++ b/dd-java-agent/instrumentation/play/play-2.3/src/main/java/datadog/trace/instrumentation/play23/RequestURIDataAdapter.java @@ -1,5 +1,8 @@ package datadog.trace.instrumentation.play23; +import static java.lang.Math.max; + +import datadog.trace.bootstrap.instrumentation.api.HostHeader; import datadog.trace.bootstrap.instrumentation.api.URIRawDataAdapter; import play.api.mvc.Request; @@ -11,9 +14,10 @@ final class RequestURIDataAdapter extends URIRawDataAdapter { RequestURIDataAdapter(Request request) { this.request = request; - int split = request.host().lastIndexOf(':'); - this.host = split == -1 ? request.host() : request.host().substring(0, split); - this.port = split == -1 ? 0 : Integer.parseInt(request.host().substring(split + 1)); + // the Host header comes from the client, so the port may be missing or malformed + final String hostHeader = request.host(); + this.host = HostHeader.host(hostHeader); + this.port = max(HostHeader.port(hostHeader), 0); } @Override diff --git a/dd-java-agent/instrumentation/play/play-2.4/src/main/java/datadog/trace/instrumentation/play24/RequestURIDataAdapter.java b/dd-java-agent/instrumentation/play/play-2.4/src/main/java/datadog/trace/instrumentation/play24/RequestURIDataAdapter.java index 0145913ed7e..864b4caee72 100644 --- a/dd-java-agent/instrumentation/play/play-2.4/src/main/java/datadog/trace/instrumentation/play24/RequestURIDataAdapter.java +++ b/dd-java-agent/instrumentation/play/play-2.4/src/main/java/datadog/trace/instrumentation/play24/RequestURIDataAdapter.java @@ -1,5 +1,8 @@ package datadog.trace.instrumentation.play24; +import static java.lang.Math.max; + +import datadog.trace.bootstrap.instrumentation.api.HostHeader; import datadog.trace.bootstrap.instrumentation.api.URIRawDataAdapter; import play.api.mvc.Request; @@ -11,9 +14,10 @@ final class RequestURIDataAdapter extends URIRawDataAdapter { RequestURIDataAdapter(Request request) { this.request = request; - int split = request.host().lastIndexOf(':'); - this.host = split == -1 ? request.host() : request.host().substring(0, split); - this.port = split == -1 ? 0 : Integer.parseInt(request.host().substring(split + 1)); + // the Host header comes from the client, so the port may be missing or malformed + final String hostHeader = request.host(); + this.host = HostHeader.host(hostHeader); + this.port = max(HostHeader.port(hostHeader), 0); } @Override diff --git a/dd-java-agent/instrumentation/play/play-2.6/src/main/java/datadog/trace/instrumentation/play26/RequestURIDataAdapter.java b/dd-java-agent/instrumentation/play/play-2.6/src/main/java/datadog/trace/instrumentation/play26/RequestURIDataAdapter.java index d2bd4ba01a1..1535353b84b 100644 --- a/dd-java-agent/instrumentation/play/play-2.6/src/main/java/datadog/trace/instrumentation/play26/RequestURIDataAdapter.java +++ b/dd-java-agent/instrumentation/play/play-2.6/src/main/java/datadog/trace/instrumentation/play26/RequestURIDataAdapter.java @@ -1,5 +1,8 @@ package datadog.trace.instrumentation.play26; +import static java.lang.Math.max; + +import datadog.trace.bootstrap.instrumentation.api.HostHeader; import datadog.trace.bootstrap.instrumentation.api.URIRawDataAdapter; import play.api.mvc.Request; @@ -11,9 +14,10 @@ final class RequestURIDataAdapter extends URIRawDataAdapter { RequestURIDataAdapter(Request request) { this.request = request; - int split = request.host().lastIndexOf(':'); - this.host = split == -1 ? request.host() : request.host().substring(0, split); - this.port = split == -1 ? 0 : Integer.parseInt(request.host().substring(split + 1)); + // the Host header comes from the client, so the port may be missing or malformed + final String hostHeader = request.host(); + this.host = HostHeader.host(hostHeader); + this.port = max(HostHeader.port(hostHeader), 0); } @Override diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/HostHeader.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/HostHeader.java new file mode 100644 index 00000000000..59341e557b7 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/HostHeader.java @@ -0,0 +1,46 @@ +package datadog.trace.bootstrap.instrumentation.api; + +import static datadog.trace.util.IntStringUtils.parseNonNegativeInt; + +import javax.annotation.Nullable; + +/** + * Splits an HTTP {@code Host} header value ({@code host}, {@code host:port}, {@code [ipv6]} or + * {@code [ipv6]:port}) without throwing. The value comes from the client, so it may be malformed. + */ +public final class HostHeader { + + private HostHeader() {} + + /** + * @return the index of the ':' separating the host from the port, or {@code -1} if there is no + * port; a ':' inside a bracketed IPv6 literal is not a separator + */ + public static int portSeparator(@Nullable String hostHeader) { + if (hostHeader == null) { + return -1; + } + final int colon = hostHeader.lastIndexOf(':'); + return colon > hostHeader.lastIndexOf(']') ? colon : -1; + } + + /** + * @return the host part, or the whole value if there is no port + */ + @Nullable + public static String host(@Nullable String hostHeader) { + final int separator = portSeparator(hostHeader); + return separator == -1 ? hostHeader : hostHeader.substring(0, separator); + } + + /** + * @return the port, or {@code -1} if there is no port or it is not a valid number + */ + public static int port(@Nullable String hostHeader) { + final int separator = portSeparator(hostHeader); + if (separator == -1) { + return -1; + } + return parseNonNegativeInt(hostHeader, separator + 1, hostHeader.length() - separator - 1); + } +} diff --git a/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/HostHeaderTest.java b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/HostHeaderTest.java new file mode 100644 index 00000000000..7610ae48b69 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/bootstrap/instrumentation/api/HostHeaderTest.java @@ -0,0 +1,35 @@ +package datadog.trace.bootstrap.instrumentation.api; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import org.junit.jupiter.api.Test; +import org.tabletest.junit.TableTest; + +class HostHeaderTest { + + @TableTest({ + "scenario | header | host | port", + "host only | example.com | example.com | -1 ", + "host and port | example.com:8080 | example.com | 8080", + "IPv4 and port | 10.0.0.1:80 | 10.0.0.1 | 80 ", + "IPv6 only | '[::1]' | '[::1]' | -1 ", + "IPv6 and port | '[::1]:8443' | '[::1]' | 8443", + "empty port | example.com: | example.com | -1 ", + "non-numeric port | example.com:http | example.com | -1 ", + "port overflow | example.com:99999999999 | example.com | -1 ", + "negative port | example.com:-1 | example.com | -1 ", + "empty | '' | '' | -1 " + }) + void splitsHostHeader(String header, String host, int port) { + assertEquals(host, HostHeader.host(header)); + assertEquals(port, HostHeader.port(header)); + } + + @Test + void nullHeader() { + assertNull(HostHeader.host(null)); + assertEquals(-1, HostHeader.port(null)); + assertEquals(-1, HostHeader.portSeparator(null)); + } +}