-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
test: add case for url.parse throwing a URIError #12135
Conversation
@@ -16,3 +16,5 @@ const url = require('url'); | |||
].forEach(function(val) { | |||
assert.throws(function() { url.parse(val); }, TypeError); | |||
}); | |||
|
|||
assert.throws(function() { url.parse('http://%E0%A4%A@fail'); }, URIError); |
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.
Can you change the last parameter to assert.throws()
to /^URIError: URI malformed$/
.
The auth property of a URL is decoded via decodeURIComponent, which can throw a URIError. The test URL here will trigger this. Adds documentation on the possible errors url.parse can throw.
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.
Still not sure if it's OK for Node.js to throw an URIError (because qsEscape
is not a global URI handling function, even it is intended as a fast alternative for encodeURIComponent
), but the test and the doc are definitely appreciated :)
The auth property of a URL is decoded via decodeURIComponent, which can throw a URIError. The test URL here will trigger this. Adds documentation on the possible errors url.parse can throw. PR-URL: #12135 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Landed in 2ff107d |
Hello, the
url.parse
function usesdecodeURIComponent
, which can throw aURIError
, to parse theauth
property of a URL. The example included here will test this path.This PR also adds documentation on the possible errors that
url.parse
can throw. I'm happy to move this to a separate PR should this make things easier/cleaner.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
test, doc