Skip to content

Avoid redundant URI object creation in WebClientUtils#36641

Merged
bclozel merged 1 commit into
spring-projects:7.0.xfrom
MintBee:fix/web-client-performance
Apr 21, 2026
Merged

Avoid redundant URI object creation in WebClientUtils#36641
bclozel merged 1 commit into
spring-projects:7.0.xfrom
MintBee:fix/web-client-performance

Conversation

@MintBee

@MintBee MintBee commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Resolve issue 36605

Problem

WebClientUtils.getRequestDescription() constructed a new URI via the
multi-arg constructor on every call just to strip query parameters for logging.
This re-parses and re-validates what the input URI already parsed.

public static String getRequestDescription(HttpMethod httpMethod, URI uri) {
if (StringUtils.hasText(uri.getQuery())) {
try {
uri = new URI(uri.getScheme(), null, uri.getHost(), uri.getPort(), uri.getPath(), null, null);
}
catch (URISyntaxException ignored) {
}
}
return httpMethod.name() + " " + uri;

Changes

Replace new URI(scheme, null, host, port, path, null, null) with direct
StringBuilder composition from the original URI's components. The
reconstruction logic follows JDK's own URI.defineString(), minus the
userInfo, query, and fragment fields — including its IPv6 bracket
handling. 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()) to
uri.getRawQuery() != null for a more performant query presence check. (no decoding needed)

Performance

JMH (JDK 21, @Fork(2), 5w+10m iterations) on the full
WebClient.get().uri(...).retrieve().toBodilessEntity().block() pipeline
with a no-op connector and a 200+ char query-heavy URI:

Version ops/s 99.9% CI
Baseline (new URI(...)) 79,985 ± 19,649 [60,335 — 99,635]
Patched (StringBuilder) 100,942 ± 18,521 [82,421 — 119,464]
Speedup 1.26×

JFR confirms: URI$Parser execution samples dropped 86%, and
getRequestDescription itself drops to 0 samples (below ~10ms threshold).

Tests

Added WebClientUtilsTests for regression testing, covering various situations.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 10, 2026
sb.append(uri.getPath());
}
return httpMethod.name() + " " + uri;
return httpMethod.name() + " " + sb;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@MintBee MintBee Apr 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please, let's remove the user info, query and fragment if any of them are present. Thanks!


@Test
void decodesPathWhenStrippingQuery() {
URI uri = URI.create("https://host/hello%20world?q=1");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could explain why the path needs decoding? I think using the raw path would be consistent with URI#toString().

@MintBee MintBee Apr 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were two reasons.

  1. Since it was for logging, I though decoded path is more readable.
  2. Controller annotations take decoded string, such as @GetMapping("/hello world"). I though it would be more consistent with the overall API.

I should've made a comment about it. I was too focused on only describing the performance parts. Thanks for pointing that @bclozel

Should I rollback and return the encoded string?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please, let's use the encoded form. It will be consistent with the URI toString and will avoid log injection / log forging vulnerabilities.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved all what you have said @bclozel. Thanks for helping me a lot! Really appreciate it.

@bclozel bclozel self-assigned this Apr 10, 2026
@bclozel bclozel added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement labels Apr 10, 2026
@bclozel bclozel removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 10, 2026
@bclozel bclozel added this to the 7.0.x milestone Apr 10, 2026
@bclozel
bclozel changed the base branch from main to 7.0.x April 21, 2026 14:50
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.
This commit also strips userInfo, query, and fragment from the
log description even if URIs contain only userInfo or
fragment (without a query).

Closes spring-projectsgh-36641

Signed-off-by: 박동윤 (Park Dong-Yun) <ehddbs7458@gmail.com>
@bclozel
bclozel force-pushed the fix/web-client-performance branch from b04bf0c to 865cf3c Compare April 21, 2026 15:28
@bclozel
bclozel merged commit 865cf3c into spring-projects:7.0.x Apr 21, 2026
1 check passed
@bclozel bclozel modified the milestones: 7.0.x, 7.0.8 Apr 21, 2026
@bclozel

bclozel commented Apr 21, 2026

Copy link
Copy Markdown
Member

Thanks for your contribution @MintBee this is now merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Long URL causes high memory allocation in WebClient calls

3 participants