FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: UriTemplate expansion reserved ("+") and fragment("#") should not encode already percent encoded parts by alexander-asenov · Pull Request #2108 · googleapis/google-http-java-client · GitHub

/ google-http-java-client Public

fix: UriTemplate expansion reserved ("+") and fragment("#") should not encode already percent encoded parts - #2108

Merged
diegomarquezp merged 7 commits into
googleapis:mainfrom
alexander-asenov:main
Sep 23, 2025
ConversationCommits (7)ChecksFiles changed
Merged

fix: UriTemplate expansion reserved ("+") and fragment("#") should not encode already percent encoded parts#2108
diegomarquezp merged 7 commits into
googleapis:mainfrom
alexander-asenov:main

Conversation

alexander-asenov commented Jun 30, 2025

Copy link
Copy Markdown
Contributor

Currently the UriTemplate::expand method re-encodes already percent encoded triplets when using reserved("+") and fragment ("#") expansions, which should not be the case according to the RFC 6570.

The fix adds a new Encoder(PercentEncodedEscaper) that escapes already percent encoded parts of the input. It takes in the constructor another Escaper, allowing extensions of already existing Escapers, without changing/breaking any existing implementations.

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #2107 ☕️

alexander-asenov requested a review from a team June 30, 2025 17:10
product-auto-label Bot added the size: m Pull request size is medium. label Jun 30, 2025

cHengstler commented Jul 7, 2025

Copy link
Copy Markdown
Contributor

Hi @suztomo,

since you've reviewed our previous contribution.
Would it be possible to consider this contribution?

suztomo commented Jul 7, 2025

Copy link
Copy Markdown
Member

@ldetmer maintains this repository these days.

alexander-asenov commented Jul 14, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Hi @ldetmer,
would you be able to take a look at this PR when you get a chance?

It aims to improve the library’s compliance with RFC 6570.
We’d love to contribute towards making it more compliant.

Let me know if there’s anything I should adjust or clarify before it can be considered.

Thanks a lot!
Alex

ldetmer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 14, 2025
yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 14, 2025

alexander-asenov commented Aug 6, 2025

Copy link
Copy Markdown
Contributor Author

Hi @ldetmer,

Just wanted to follow up on the RFC 6570 compliance.

I know you're probably busy, so no rush at all! I just wanted to make sure it didn't get lost in the shuffle. If you need any additional context or have questions about the changes, I'm happy to provide more details. As I mentioned before we'd love to contribute towards making the library more compliant with the RFC.

Also, if there are any preliminary concerns you'd like me to address first, just let me know.

Thanks again for your time!
Alex

diegomarquezp added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 29, 2025
yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 29, 2025
diegomarquezp reviewed Sep 18, 2025
View reviewed changes

diegomarquezp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM in general, given the following observations I'll leave here for other reviewers:

  • new behavior indeed does not affect existing one
  • confirmed compliance with the specification

However, support for {#var} and {+var} belong to level 2 templates, while we state we support "Level 1 templates and Level 4 composite templates" in

google-http-java-client/google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java

Line 32 in 5d0689a

* <p>This Class supports Level 1 templates and all Level 4 composite templates as described in: <a

Would you mind clarifying this in that comment? Also, it could be useful to update #2107 as a feature request to support Level 2 templates.

Comment thread google-http-client/src/test/java/com/google/api/client/http/UriTemplateTest.java
diegomarquezp added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 19, 2025
yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 19, 2025
diegomarquezp merged commit 30766a8 into googleapis:main Sep 23, 2025
21 of 22 checks passed
release-please Bot mentioned this pull request Sep 24, 2025
Merged
gcf-merge-on-green Bot pushed a commit that referenced this pull request Sep 26, 2025
chore(main): release 2.0.1 (#2117)
c0af9bb 
🤖 I have created a release *beep* *boop*
---


## [2.0.1](https://togithub.com/googleapis/google-http-java-client/compare/v2.0.0...v2.0.1) (2025-09-24)


### Bug Fixes

* UriTemplate expansion reserved ("+") and fragment("#") should not encode already percent encoded parts ([#2108](https://togithub.com/googleapis/google-http-java-client/issues/2108)) ([30766a8](https://togithub.com/googleapis/google-http-java-client/commit/30766a8a74df49c37e80ec41f1021d4ad69a8fda))

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

blakeli0 blakeli0 approved these changes
+1 more reviewer
diegomarquezp diegomarquezp approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size: m Pull request size is medium.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

UriTemplate expansion reserved ("+") and fragment("#") should not encode already percent encoded parts

7 participants

Back | FazBrowse Home | New Git URL