Skip to content

proxy: preserve trailing slash through path.Join and strip_path#1272

Closed
c-tonneslan wants to merge 1 commit into
ory:masterfrom
c-tonneslan:fix/preserve-trailing-slash
Closed

proxy: preserve trailing slash through path.Join and strip_path#1272
c-tonneslan wants to merge 1 commit into
ory:masterfrom
c-tonneslan:fix/preserve-trailing-slash

Conversation

@c-tonneslan

@c-tonneslan c-tonneslan commented May 18, 2026

Copy link
Copy Markdown

Closes #1266.

The switch to path.Join in ConfigureBackendURL drops a trailing slash from the proxied path. Combined with the existing strip_path strings.Replace, a request to /some/path/ with strip_path: /some ended up at /path on the upstream instead of /path/.

Remember whether the inbound path had a trailing slash and add it back after the strip step. Two regression cases added to TestConfigureBackendURL.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue where trailing slashes in request paths were not being preserved during URL forwarding.
  • Tests

    • Added test cases to verify proper handling of trailing slash preservation in path configurations.

Review Change Stack

Closes ory#1266.

The switch to path.Join in ConfigureBackendURL drops any trailing
slash from the proxied path. Combined with the existing strip_path
strings.Replace, a request to /some/path/ with strip_path /some
ended up at /path on the upstream instead of /path/. Remember
whether the inbound path had a trailing slash and put it back after
the strip step.

Signed-off-by: Charlie Tonneslan <cst0520@gmail.com>
@c-tonneslan
c-tonneslan requested review from a team and aeneasr as code owners May 18, 2026 16:50
@CLAassistant

CLAassistant commented May 18, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented May 18, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 71d4b8e8-2537-4cab-b277-742fdaf3bfd9

📥 Commits

Reviewing files that changed from the base of the PR and between 18212fc and 71f85ec.

📒 Files selected for processing (2)
  • proxy/proxy.go
  • proxy/proxy_test.go

📝 Walkthrough

Walkthrough

This PR restores trailing slash semantics in upstream requests that were inadvertently lost when path.Join was introduced in version 26.2. The ConfigureBackendURL function now preserves trailing slashes by detecting them before path manipulation and re-applying them afterward.

Changes

Trailing Slash Preservation in Proxy Path Forwarding

Layer / File(s) Summary
Trailing slash preservation in ConfigureBackendURL
proxy/proxy.go, proxy/proxy_test.go
ConfigureBackendURL detects non-root trailing slashes in inbound paths, applies path.Join and StripPath removal, then conditionally restores the trailing slash. Two test cases validate the behavior for both /some-prefixed and api-style StripPath forms.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: preserving trailing slashes in the proxy path handling through path.Join and strip_path logic.
Description check ✅ Passed The description clearly explains the bug (trailing slash removal after upgrading), references the linked issue (#1266), describes the solution, and mentions the addition of regression tests.
Linked Issues check ✅ Passed The PR successfully addresses the primary objective from #1266: preserving trailing slashes in proxied paths by detecting inbound trailing slashes and re-appending them after strip_path processing.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the issue: modifying ConfigureBackendURL to preserve trailing slashes and adding corresponding regression test cases to TestConfigureBackendURL.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@xqqp

xqqp commented May 19, 2026

Copy link
Copy Markdown

I posted a similar MR (#1269) last month, but got no reaction from the team. I'm hoping the maintainers recognize this problem soon.

@c-tonneslan

Copy link
Copy Markdown
Author

Thanks @xqqp, and sorry, I didn't spot #1269 when I opened this. It's the same fix and a month older, so closing mine in favour of yours.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Oathkeeper 26.2 strips trailing slash

3 participants