Skip to content
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

Handle query parameters in FileMiddleware redirects #3077

Merged
merged 3 commits into from Oct 11, 2023

Conversation

Captain-Kirkie
Copy link
Contributor

@Captain-Kirkie Captain-Kirkie commented Oct 1, 2023

Correctly handle query parameters when using the redirect functionality in FileMiddleware.

@Captain-Kirkie
Copy link
Contributor Author

Issue Here

Copy link
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! One change then we can merge

Sources/Vapor/Middleware/FileMiddleware.swift Outdated Show resolved Hide resolved
@0xTim 0xTim added the semver-patch Internal changes only label Oct 5, 2023
@0xTim 0xTim changed the title bug: fix redirection with query parameters. Handle query parameters in FileMiddleware redirects Oct 5, 2023
@Captain-Kirkie Captain-Kirkie force-pushed the bug/query-params branch 2 times, most recently from 7d8f5c5 to 479d052 Compare October 7, 2023 02:44
gwynne
gwynne previously requested changes Oct 7, 2023
Sources/Vapor/Middleware/FileMiddleware.swift Outdated Show resolved Hide resolved
fix: redirect to entire path

refactor: updating redirection path without string manipulation.
@codecov-commenter
Copy link

codecov-commenter commented Oct 7, 2023

Codecov Report

Merging #3077 (fabcd05) into main (61c8104) will increase coverage by 0.12%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3077      +/-   ##
==========================================
+ Coverage   76.22%   76.34%   +0.12%     
==========================================
  Files         211      211              
  Lines        7862     7864       +2     
==========================================
+ Hits         5993     6004      +11     
+ Misses       1869     1860       -9     
Files Coverage Δ
Sources/Vapor/Middleware/FileMiddleware.swift 94.73% <100.00%> (+0.14%) ⬆️

... and 2 files with indirect coverage changes

Copy link
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

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

Thanks!

@0xTim 0xTim enabled auto-merge (squash) October 11, 2023 12:19
@0xTim 0xTim merged commit 288d73a into vapor:main Oct 11, 2023
16 checks passed
@penny-for-vapor
Copy link

These changes are now available in 4.84.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-patch Internal changes only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants