Skip to content

fix: prevent URI percent-decoding in c8y_auth_proxy path handling#3886

Merged
rina23q merged 1 commit intothin-edge:mainfrom
rina23q:fix/3884/c8y-http-proxy-decode-escape-chars
Dec 3, 2025
Merged

fix: prevent URI percent-decoding in c8y_auth_proxy path handling#3886
rina23q merged 1 commit intothin-edge:mainfrom
rina23q:fix/3884/c8y-http-proxy-decode-escape-chars

Conversation

@rina23q
Copy link
Copy Markdown
Member

@rina23q rina23q commented Dec 2, 2025

Proposed changes

axum::extract::Path forces any percent encoded parameters decoded, that causes a url containing encoded semicolon %3B changed into decoded ;, and c8y responses with an error.

Any percent encoded parameters will be automatically decoded. The decoded parameters must be valid UTF-8, otherwise Path will fail and return a 400 Bad Request response.

This PR uses a workaround to avoid using axum::extract::Path.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

#3884

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s. You can activate automatic signing by running just prepare-dev once)
  • I ran just format as mentioned in CODING_GUIDELINES
  • I used just check as mentioned in CODING_GUIDELINES
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 2, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 2, 2025

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
745 0 3 745 100 2h27m41.132381s

Copy link
Copy Markdown
Contributor

@jarhodes314 jarhodes314 left a comment

Choose a reason for hiding this comment

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

Nice fix. A couple of suggested improvements, but the changes are broadly good.

While we're fixing this, I suspect we've got a similar potential bug with the handling of query parameters, it would be good to check if that's the case and handle it similarly if so.

@reubenmiller reubenmiller added the theme:c8y Theme: Cumulocity related topics label Dec 3, 2025
@rina23q rina23q requested a review from reubenmiller as a code owner December 3, 2025 14:28
@rina23q rina23q temporarily deployed to Test Pull Request December 3, 2025 14:45 — with GitHub Actions Inactive
Copy link
Copy Markdown
Contributor

@didier-wenzek didier-wenzek left a comment

Choose a reason for hiding this comment

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

Approved

Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>
@rina23q rina23q force-pushed the fix/3884/c8y-http-proxy-decode-escape-chars branch from 65eadf7 to 2cbe306 Compare December 3, 2025 15:20
@rina23q rina23q temporarily deployed to Test Pull Request December 3, 2025 15:20 — with GitHub Actions Inactive
@rina23q rina23q added this pull request to the merge queue Dec 3, 2025
Merged via the queue into thin-edge:main with commit 5031b06 Dec 3, 2025
34 checks passed
@rina23q rina23q deleted the fix/3884/c8y-http-proxy-decode-escape-chars branch December 3, 2025 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

theme:c8y Theme: Cumulocity related topics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants