Skip to content

Fix Play server spans losing URL tags on a malformed Host port #12790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
dougqh wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Fix Play server spans losing URL tags on a malformed Host port #12790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
252857a
Fix Oracle and jTDS JDBC URL parsing failures
dougqh Oct 7, 2026
6c8ab81
Share parsePort between the Oracle and jTDS URL parsers
dougqh Oct 7, 2026
8347145
Parse JDBC URL ports without exceptions
dougqh Oct 7, 2026
977b61e
Take (start, len) in Numbers.parseNonNegativeInt to match LongStringU…
dougqh Oct 7, 2026
a97eccf
Move parseNonNegativeInt into IntStringUtils
dougqh Oct 7, 2026
18793b5
Parse the Play Host header port without exceptions
dougqh Oct 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 92 additions & 64 deletions ...p/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package datadog.trace.bootstrap.instrumentation.jdbc;

import static datadog.trace.bootstrap.instrumentation.jdbc.DBInfo.DEFAULT;
import static datadog.trace.util.IntStringUtils.parseNonNegativeInt;
import static java.lang.Math.max;

import datadog.trace.api.Pair;
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -390,31 +385,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, 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, 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;
Expand All @@ -428,7 +429,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;
Expand All @@ -447,6 +448,20 @@ DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) {
}
return builder.instance(instance);
}

/** 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() {
Expand All @@ -469,10 +484,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;
}
Expand Down Expand Up @@ -511,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);
Expand Down Expand Up @@ -697,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;
}
Expand Down Expand Up @@ -726,6 +743,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:<subtype>://" prefix to parse
return builder;
}
final String subtype = jdbcUrl.substring(typeEndLoc + 1, protoLoc);

builder.subtype(subtype);
Expand All @@ -740,40 +761,34 @@ DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) {
}
}

// <server>[:<port>][/<database>][;<property>=<value>[;...]]
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());
try {
builder.port(Integer.parseInt(details.substring(portLoc + 1, portEndLoc)));
} catch (final NumberFormatException ignored) {
}
} else if (dbLoc > 0) {
hostEndLoc = dbLoc;
} else if (paramLoc > 0) {
hostEndLoc = paramLoc;
final int dbLoc = address.indexOf('/');
final String hostAndPort;
if (dbLoc >= 0) {
builder.db(address.substring(dbLoc + 1));
hostAndPort = address.substring(0, dbLoc);
} else {
hostEndLoc = details.length();
hostAndPort = address;
}

builder.host(details.substring(0, hostEndLoc));
final int portLoc = hostAndPort.indexOf(':');
if (portLoc >= 0) {
setPort(builder, parsePort(hostAndPort, portLoc + 1, hostAndPort.length()));
builder.host(hostAndPort.substring(0, portLoc));
} else {
builder.host(hostAndPort);
}

return builder;
}
Expand Down Expand Up @@ -900,6 +915,29 @@ public static DBInfo parse(String connectionUrl, final Properties props) {

// Source: https://stackoverflow.com/a/13592567
@SuppressForbidden
/**
* @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 - start);
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);
}
}

private static Map<String, String> splitQuery(final String query, final char separator) {
if (query == null || query.isEmpty()) {
return Collections.emptyMap();
Expand Down Expand Up @@ -956,25 +994,15 @@ 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
builder.instance((String) props.get("servicename"));
}

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")));
}
}
}
Expand Down
36 changes: 36 additions & 0 deletions ...st/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParserJtdsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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:<type>://<server>[:<port>][/<db>][;<props>]}).
*
* <p>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());
}
}
39 changes: 39 additions & 0 deletions .../java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParserOracleTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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}.
*
* <p>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());
}
}
10 changes: 7 additions & 3 deletions ...ay/play-2.3/src/main/java/datadog/trace/instrumentation/play23/RequestURIDataAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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
Expand Down
Loading
Loading