-
Notifications
You must be signed in to change notification settings - Fork 543
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
Add default "time" param to parsed form (if any) #3599
Conversation
If the form was already parsed by a downstream middleware, we need to add it to that form too, otherwise it won't be available to upstream by just adding it to the url query. Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
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.
Thanks, this looks good to me. (But I'm not sure if it will help to solve problem in #3561 -- we will need to "reset" body in that PR, right?)
r.URL.RawQuery = q.Encode() | ||
|
||
// If form was already parsed, add this param to the form too. |
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.
We should also mention that if form already had time
parameter, this extra time
value will be ignored.
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.
What I did was to add back the r.Form.Has("time")
guard in this method, as otherwise we were adding the "time" to the form even if it already had it.
It only worked because I used the "Add" method, which added another value, and the old value was still being read.
Changed this and added a test case for that: a69fb16
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.
Sounds good. Thanks.
Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
* Add default "time" param to parsed form (if any) If the form was already parsed by a downstream middleware, we need to add it to that form too, otherwise it won't be available to upstream by just adding it to the url query. Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Check that form doesn't have "time" Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
* Add default "time" param to parsed form (if any) If the form was already parsed by a downstream middleware, we need to add it to that form too, otherwise it won't be available to upstream by just adding it to the url query. Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Check that form doesn't have "time" Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
What this PR does
If the form was already parsed by a downstream middleware, we need to add it to that form too, otherwise it won't be available to upstream by just adding it to the url query.
This implement's @pstibrany's suggestion #3588 (comment) and copies all the tests from that PR.
Which issue(s) this PR fixes or relates to
Will make #3561 easier to review.
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]