Avoid redundant URI object creation in WebClientUtils#36641
Avoid redundant URI object creation in WebClientUtils#36641MintBee wants to merge 2 commits intospring-projects:mainfrom
Conversation
Prior to this commit, WebClientUtils.getRequestDescription() created a new URI object on every invocation. Since the URI constructor includes validation and parsing, which is already performed by the parameter URI object, this was unnecessarily expensive for a logging utility. This commit reuses the original URI's string representation to avoid redundant parsing. Closes spring-projectsgh-36605 Signed-off-by: 박동윤 (Park Dong-Yun) <ehddbs7458@gmail.com>
spring-projects-issues
added
the
status: waiting-for-triage
Reduce nesting in query-string handling by returning early when rawQuery is null, improving readability without changing behavior. Signed-off-by: 박동윤 (Park Dong-Yun) <ehddbs7458@gmail.com>
| sb.append(uri.getPath()); | ||
| } | ||
| return httpMethod.name() + " " + uri; | ||
| return httpMethod.name() + " " + sb; |
There was a problem hiding this comment.
Could we consider adding httpMethod.name() and the " " at the beginning to the StringBuilder directly? Using a Srting builder to then perform two concatenations is a bit wasteful.
|
|
||
| @Test | ||
| void preservesUserInfoWithoutQuery() { | ||
| URI uri = URI.create("https://admin:secret@host/api"); |
There was a problem hiding this comment.
in the original issue, you added a comment about this: I think we should indeed strip the user info from the logged URI. Can you update the implementation to check for the presence of raw user info?
There was a problem hiding this comment.
Should I strip the fragment too when only the fragment is in presence for the URI? I guess we should, since this is more consistent with existing API, and the purpose of the method is purely for logging.
|
|
||
| @Test | ||
| void decodesPathWhenStrippingQuery() { | ||
| URI uri = URI.create("https://host/hello%20world?q=1"); |
There was a problem hiding this comment.
Could explain why the path needs decoding? I think using the raw path would be consistent with URI#toString().
bclozel
added
in: web
bclozel
removed
the
status: waiting-for-triage
3 participants
Resolve issue 36605
Problem
WebClientUtils.getRequestDescription()constructed a newURIvia themulti-arg constructor on every call just to strip query parameters for logging.
This re-parses and re-validates what the input
URIalready parsed.spring-framework/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientUtils.java
Lines 75 to 83 in 2ee4c3a
Changes
Replace
new URI(scheme, null, host, port, path, null, null)with directStringBuildercomposition from the original URI's components. Thereconstruction logic follows JDK's own
URI.defineString(), minus theuserInfo,query, andfragmentfields — including its IPv6 brackethandling. Since the URI spec (RFC 3986) has been stable since 2005, this
component-to-string mapping is unlikely to break.
Also switched from
StringUtils.hasText(uri.getQuery())touri.getRawQuery() != nullfor a more performant query presence check. (no decoding needed)Performance
JMH (JDK 21,
@Fork(2), 5w+10m iterations) on the fullWebClient.get().uri(...).retrieve().toBodilessEntity().block()pipelinewith a no-op connector and a 200+ char query-heavy URI:
new URI(...))StringBuilder)JFR confirms:
URI$Parserexecution samples dropped 86%, andgetRequestDescriptionitself drops to 0 samples (below ~10ms threshold).Tests
Added
WebClientUtilsTestsfor regression testing, covering various situations.