Skip to content

Strip query string from relative document URLs in Swagger UI - #4152

Merged
martincostello merged 3 commits into
domaindrivendev:masterfrom
GabeSilvaDev:fix/2721-strip-query-from-document-url
Sep 15, 2026
Merged

martincostello merged 3 commits into
domaindrivendev:masterfrom
GabeSilvaDev:fix/2721-strip-query-from-document-url

Conversation

@GabeSilvaDev

@GabeSilvaDev GabeSilvaDev commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Pull Request

The issue or feature being addressed

Fixes #2721

Details on the issue fix or feature implementation

The Swagger UI initializer script (index.js) resolves relative document URLs against window.location.href and only stripped the fragment, so a request for swagger/index.html?foo=bar produced a document link of swagger/swagger.json?foo=bar. This is what scanners such as Burp Suite flag as a reflected open-redirect finding, as described in the issue.

The page URL now has its query string and fragment removed (split(/[?#]/)[0]) before the document URL is spliced in, so nothing from the page URL is carried over. The document URL itself is treated exactly as before: a configured query string such as v1/swagger.json?tenant=foo is kept, and only a fragment is dropped. Absolute and root-relative document URLs are not touched.

Added an integration test in SwaggerUIIntegrationTests (same style as SwaggerUIMiddleware_ReturnsInitializerScript) asserting the served script strips the page URL before the replacement; it failed before the change.

The initializer script resolves relative document URLs against
window.location.href and only removed the fragment, so a request for
index.html?foo=bar produced a document link of swagger.json?foo=bar.
Drop the query string as well.
Copilot AI lite review requested due to automatic review settings September 11, 2026 17:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Configured endpoint query parameters are currently removed during URL normalization.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates Swagger UI relative document URL resolution to remove page query strings and fragments.

Changes:

  • Adjusts URL normalization in index.js.
  • Adds an integration test for the served initializer script.
File summaries
File Description
test/Swashbuckle.AspNetCore.IntegrationTests/SwaggerUIIntegrationTests.cs Tests the updated initializer script behavior.
src/Swashbuckle.AspNetCore.SwaggerUI/index.js Normalizes relative document URLs.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Swashbuckle.AspNetCore.SwaggerUI/index.js Outdated
Splitting after the replacement also removed a query string configured
on the document URL itself (e.g. v1/swagger.json?tenant=foo). Drop the
page's query string and fragment first, then splice in the document URL.
@codecov

codecov Bot commented Sep 13, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.24%. Comparing base (33f5bf2) to head (c0021c0).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4152      +/-   ##
==========================================
+ Coverage   95.22%   95.24%   +0.01%     
==========================================
  Files         111      111              
  Lines        4128     4142      +14     
  Branches      845      848       +3     
==========================================
+ Hits         3931     3945      +14     
  Misses        197      197              
Flag Coverage Δ
Linux 95.24% <ø> (+0.01%) ⬆️
Windows 95.24% <ø> (+0.01%) ⬆️
macOS 95.24% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread test/Swashbuckle.AspNetCore.IntegrationTests/SwaggerUIIntegrationTests.cs Outdated
The integration test only checked that the served index.js contained the new expression, which does not exercise the behaviour the change is meant to fix.

Load swagger/index.html?foo=bar in Chromium against the TopLevelSwaggerDoc site, whose document URL is the relative v1.json, and assert that the document URL Swagger UI shows in the info panel ends with /swagger/v1.json. Against the previous index.js the same test fails with .../swagger/v1.json?foo=bar.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The implementation preserves configured document queries while addressing the reported behavior with regression coverage.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@martincostello martincostello added this to the v10.2.4 milestone Sep 15, 2026
@martincostello
martincostello merged commit cb65abd into domaindrivendev:master Sep 15, 2026
14 checks passed
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Thanks for your contribution @GabeSilvaDev - the changes from this pull request have been published as part of version 10.3.0 📦, which is now available from NuGet.org 🚀

This was referenced Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Open redirect finding on request for index.html with query parameters

3 participants