Fix 3 bugs: no-arg redirect, cookie option leak, view render context.#7273
Open
Mohammad-Faiz-Cloud-Engineer wants to merge 1 commit into
Open
Fix 3 bugs: no-arg redirect, cookie option leak, view render context.#7273Mohammad-Faiz-Cloud-Engineer wants to merge 1 commit into
Mohammad-Faiz-Cloud-Engineer wants to merge 1 commit into
Conversation
While digging through the response and view code, found a few things that needed fixing: 1. res.redirect() with no args - if you call it without a URL, it'd redirect to the literal string "undefined". The deprecation warning was there but didn't stop it from doing something broken. Added a safe fallback to '/'. 2. res.cookie() leaked opts.signed - the signed property was being read from options but never cleaned up before passing the whole object to cookie.serialize(). The cookie package ignores unknown keys so it works today, but it's a leak waiting to bite if that ever changes. 3. View.prototype.render inconsistent this - the sync and async paths used different this values when calling back. Both come from wherever the template engine sets it, but they could differ between the first call and subsequent calls. Unified it so both paths use the same captured context.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found a few things that needed fixing:
res.redirect() with no args - if you call it without a URL, it'd redirect to the literal string "undefined". The deprecation warning was there but didn't stop it from doing something broken. Added a safe fallback to '/'.
res.cookie() leaked opts.signed - the signed property was being read from options but never cleaned up before passing the whole object to cookie.serialize(). The cookie package ignores unknown keys so it works today, but it's a leak waiting to bite if that ever changes.
View.prototype.render inconsistent this - the sync and async paths used different this values when calling back. Both come from wherever the template engine sets it, but they could differ between the first call and subsequent calls. Unified it so both paths use the same captured context.