Repository navigation
Fix DB2 JDBC URL parsing failure when :// is only in a property value (quick fix) - #12803
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da7f3d0fff
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
sarahchen6
left a comment
There was a problem hiding this comment.
The codex/dd review looks worth addressing, but otherwise LGTM
There was a problem hiding this comment.
Previously accepted ports such as +1444 now fall back to vendor defaults, producing incorrect JDBC connection metadata. The inline replacement preserves that compatibility without changing the integer utility's digits-only contract.
🤖 Bits Code Review · Commit f39cdd7
A DB2/AS400 URL without // after the type (e.g. jdbc:as400:host;keystore=file:///x) whose only :// sits inside a ';' property value made MODIFIED_URL_LIKE take the '://' as the host delimiter while cutting urlPart1 at the earlier ';', so urlPart1.substring(hostIndex + 3) threw StringIndexOutOfBoundsException. Return the builder unchanged in that case, which is the same DBInfo the exception fallback produced, minus the exception. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The early return skipped splitQuery/populateStandardProperties, so properties such as user or schema were lost. The old parser applied them before throwing and parse() returned that builder from its catch block, so populate them before returning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f39cdd7 to
07713f9
Compare
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
What Does This Do
Stops
JDBCConnectionUrlParserfrom throwingStringIndexOutOfBoundsExceptionon DB2/AS400 URLs that have no//after the type and whose only://is inside a;property value, for example:jdbc:as400:host;ssltruststore=file://xjdbc:as400:host;libraries=a;secure=true;keystore=file:///xjdbc:db2:mydb;x=http://yMODIFIED_URL_LIKEtook that://as the host delimiter. The type then wasn'tdb2/as400, so the code went down the generic;branch and cuturlPart1at the earlier;. That left it shorter thanhostIndex + 3, andurlPart1.substring(hostIndex + 3)threw. Now, when the first;comes before://, the parser returns the builder unchanged. That's the sameDBInfothe exception fallback inparsealready produced (type and default port 50000, no host), just without the exception. URLs that parsed before still parse the same way.Motivation
Error Tracking issue https://app.datadoghq.com/error-tracking/issue/ab130d54-aab1-11f0-80ae-da7ad0900002 ("Error parsing URL", ~3.3k errors/day, still alerting on 1.66):
#11550 (1.64.0) fixed one cause, the
lastIndexOf(':')landing inside://. But the monitor kept firing on 1.66. I ran a wide set of DB2/AS400 URL forms throughDB2.doParseon current code, and the form above was the only one that still threw.Additional Notes
JDBCConnectionUrlParserDB2Test. The test callsDB2.doParsedirectly becauseextractDBInfoswallows exceptions. All 4 rows fail without the fix and pass with it.:dd-java-agent:agent-bootstrap:test --tests '*JDBCConnectionUrlParser*',:dd-java-agent:instrumentation:jdbc:test --tests JDBCConnectionUrlParserTest,spotlessJavaCheckandforbiddenApisMain. All passed.🤖 Generated with Claude Code