Follow redirects with encoded URI #564
Conversation
- add arguments test for query with special chars - add redirects test for uri-encoded location with special chars - redirects are handled with a recursive call to `get`, so deferred recursion with setImmediate - test server enhancement: - now replies with 404 if no listener - decodes request.url before emission
jstewmon
force-pushed
the
query-escapes
branch
from
c8b9704
to
67fa93d
Compare
jstewmon
changed the title
|
@sindresorhus IMO it's ready to merge. https://github.com/sindresorhus/got/pull/564/files#diff-62bdc57f6f22ae58f495daef16f21f8bR93 is not needed, because the function is called when |
| @@ -74,7 +74,7 @@ module.exports = options => { | |||
| redirectUrl = (new URLGlobal(bufferString, urlLib.format(options))).toString(); | |||
There was a problem hiding this comment.
I think using urlLib.format to get the baseUrl has the same problem as was discussed in #519 (comment).
I can take a look at adding a test and fixing today.
There was a problem hiding this comment.
The baseUrl won’t have the right query string, but I don’t think there’s any value for new url for which it would affect the result. So this seems fine.
See my comment above.
Could this be placed in a separate PR please? |
sindresorhus
changed the title
I've reverted this: 731c470
I agree this should have been a separate change, but we decided we can live with it as we want to get out a new minor release today, and it's just nitpick. |
|
Thanks for fixing this, @jstewmon 🙌 |
Fixes #563