fix: properly encode url with unicode characters #1291
Conversation
Hmmm 🤔 no idea why that happened... |
LinusU
force-pushed
the
issue-1290
branch
2 times, most recently
from
e80c3f7
to
cd81de3
Compare
|
@jimmywarting pushed a commit for that 👍 |
|
@jimmywarting I don't have permission to publish to npm, would you be able to? I have pushed a |
|
Yes i have permission |
|
Released 2.6.3 to npm and github release. |
|
This PR caused a breaking change to the encoding behavior of URLs that breaks, for example, the proper formatting of SAS tokens in URLs in |
|
@dmichon-msft sorry, I missed your comment here. As you are probably already aware this was fixed in |
| @@ -36,6 +36,9 @@ | |||
| "url": "https://github.com/bitinn/node-fetch/issues" | |||
| }, | |||
| "homepage": "https://github.com/bitinn/node-fetch", | |||
| "dependencies": { | |||
| "whatwg-url": "^5.0.0" | |||
There was a problem hiding this comment.
why this version?
actual is 9.1.0
this breaks our code because we have dependency on whatwg-url v.6.5.0 already
There was a problem hiding this comment.
why this version?
actual is 9.1.0
This is the latest version that supports Node.js 4.x
this breaks our code because we have dependency on whatwg-url v.6.5.0 already
In which way does your app break? 🤔
There was a problem hiding this comment.
This also breaks my app : for security reasons, we use the --throw-deprecation and --pending-deprecation node flags, and whatwg-url appears to use Buffer() : I get
DeprecationWarningcode: DEP0005 DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
at showFlaggedDeprecation (buffer.js:194:11)
at new Buffer (buffer.js:278:3)
at utf8PercentDecode (/project/node_modules/whatwg-url/lib/url-state-machine.js:105:17)
at parseHost (/project/node_modules/whatwg-url/lib/url-state-machine.js:400:18)
at URLStateMachine.parseHostName (/project/node_modules/whatwg-url/lib/url-state-machine.js:851:18)
at new URLStateMachine (/project/node_modules/whatwg-url/lib/url-state-machine.js:562:44)
at Object.module.exports.basicURLParse (/project/node_modules/whatwg-url/lib/url-state-machine.js:1258:15)
at new URLImpl (/project/node_modules/whatwg-url/lib/URL-impl.js:17:27)
at Object.setup (/project/node_modules/whatwg-url/lib/URL.js:187:17)
at new URL (/project/node_modules/whatwg-url/lib/URL.js:25:18)
There was a problem hiding this comment.
I've asked upstream if we can create a 5.0.1 version of whatwg-url that avoids the deprecation warning here: jsdom/whatwg-url#216
But apart from that I'm not really sure how to handle using deprecated APIs 🤔 I don't think that I would consider it a breaking chance, but I'm not sure...
What is the purpose of this pull request?
What changes did you make? (provide an overview)
I've added a call to
encodeURIbefore parsing a passed in string as an urlWhich issue (if any) does this pull request address?
#1290
Is there anything you'd like reviewers to know?
This is basically a backport of #701, but keeps the old code path for arbitrary URLs on the
Requestobject which was supported in 2.x