Skip to content

Refine UriUtils#decode and StringUtils#uriDecode implementation and documentation #34673

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.

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

Closed

Conversation

kilink
Copy link
Contributor

@kilink kilink commented Mar 28, 2025

Optimize the StringUtils.uriDecode method in the following ways:

  • Use a StringBuilder instead of ByteArrayOutputStream, and only decode %-encoded sequences.
  • Use HexFormat.fromHexDigits to decode hex sequences.
  • Decode to a byte array that is only allocated if encoded sequences are encountered.

Using a StringBuilder and decoding to a temporary byte buffer are inspired by how JDK's URLDecoder works; HexFormat was added in JDK17 and should be more performant than Integer.parseInt as it uses lookup tables.

I initially looked at this as an easy optimization opportunity, but it appears this also resolves the issue raised in #34570; I've added a test case to exercise the problem described in that issue.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 28, 2025
@kilink kilink force-pushed the optimize-stringutils-uridecode branch from eec755b to 037a4ec Compare March 28, 2025 19:15
@sdeleuze sdeleuze self-assigned this Mar 29, 2025
@sdeleuze sdeleuze added in: web Issues in web modules (web, webmvc, webflux, websocket) in: core Issues in core modules (aop, beans, core, context, expression) labels Mar 29, 2025
@sdeleuze sdeleuze added this to the 7.0.0-M4 milestone Mar 29, 2025
@sdeleuze sdeleuze added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 29, 2025
Copy link
Contributor

@sdeleuze sdeleuze left a comment

Choose a reason for hiding this comment

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

@kilink Could you please add a Signed-off-by trailer to your commit message as described in https://spring.io/blog/2025/01/06/hello-dco-goodbye-cla-simplifying-contributions-to-spring?

@sdeleuze sdeleuze added the status: waiting-for-feedback We need additional information before we can continue label Apr 1, 2025
Optimize the StringUtils.uriDecode method in the following ways:

- Use a StringBuilder instead of ByteArrayOutputStream, and only decode %-encoded sequences.
- Use HexFormat.fromHexDigits to decode hex sequences.
- Decode to a byte array that is only allocated if encoded sequences are encountered.

Signed-off-by: Patrick Strawderman <pstrawderman@netflix.com>
@kilink kilink force-pushed the optimize-stringutils-uridecode branch from 037a4ec to f3e2a6a Compare April 1, 2025 18:44
@kilink
Copy link
Contributor Author

kilink commented Apr 1, 2025

@kilink Could you please add a Signed-off-by trailer to your commit message as described in https://spring.io/blog/2025/01/06/hello-dco-goodbye-cla-simplifying-contributions-to-spring?

Done.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Apr 1, 2025
@sdeleuze sdeleuze changed the title Optimize StringUtils#uriDecode Refine UriUtils#decode and StringUtils#uriDecode implementation and documentation Apr 2, 2025
sdeleuze pushed a commit to sdeleuze/spring-framework that referenced this pull request Apr 2, 2025
Refine the StringUtils#uriDecode method in the following ways:

- Use a StringBuilder instead of ByteArrayOutputStream, and only decode
  %-encoded sequences.
- Use HexFormat.fromHexDigits to decode hex sequences.
- Decode to a byte array that is only allocated if encoded sequences are
  encountered.

Signed-off-by: Patrick Strawderman <pstrawderman@netflix.com>
See spring-projectsgh-34673
@sdeleuze sdeleuze closed this in dd888ed Apr 2, 2025
@sdeleuze
Copy link
Contributor

sdeleuze commented Apr 2, 2025

Merged with some additional optimizations + a Javadoc update, thanks a lot @kilink.

@sdeleuze sdeleuze removed the status: feedback-provided Feedback has been provided label Apr 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) 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.

4 participants