-
Notifications
You must be signed in to change notification settings - Fork 357
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
fix: stop pass through request params for function redirects #4897
fix: stop pass through request params for function redirects #4897
Conversation
📊 Benchmark resultsComparing with ec1a05e Package size: 222 MB(no change)
Legend
|
Note: I confirmed w/ https://github.com/eric-silverman on Slack for approval before creating the PR for the attached issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉
Can we add an additional assertion that tests the :splat
behavior with functions?
Yeah. I'll add a test-case. |
Feel free to merge. You can also use kodiak for merging, by simply adding the |
@danez I don't have access to merge or add any labels yet. |
Oh I see, I can merge if you want? |
Yeah. Please do. |
🎉 Thanks for submitting a pull request! 🎉
Summary
Fixes #4273
Query parameters specified in function redirects are not being passed (they're over-written by the request parameters) by
netlify dev
. This is different from currently observed production behaviour.This is due to an intentional change in #1605 to match the production behaviour at that time.
Difference b/w current local & production behaviour can be verified with the minimal reproduction & production deploy link below:
Minimal reproduction of the issue: https://github.com/KK-Learning-Gym/4273-with-fn (see netlify.toml)
Production Deploy:
https://with-fn-testing.netlify.app
Quick Links:
Redirect with config from the updated test in this PR:
https://with-fn-testing.netlify.app/api/test?foo=1&foo=2&bar=1&bar=2
Redirects to
/.netlify/functions/echo?a=1&a=2
on production/.netlify/functions/echo?foo=1&foo=2&bar=1&bar=2
locallyRedirect with config mentioned in #4273 on production:
https://with-fn-testing.netlify.app/.netlify/functions/bar/?url=bar
Redirects to
.netlify/functions/bar/?url=bar
on production.netlify/functions/bar/?url=
locallyAlso see #4273 (comment)
This PR attempts to resolve the differences described above b/w
netlify dev
& production.For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)
🦁