Check unicode links correctly #3
Conversation
|
Thank you! I'm working on a different task at the moment, but will come back to this soon. For the time being, I have a couple of questions about your changes and will leave them inline. Appreciate the help! :) |
| @@ -50,7 +50,7 @@ function runTest(options, callback) { | |||
| function testOutput(test, log, error, exception) { | |||
| return function(err, context, count) { | |||
| if (err || exception) { | |||
| test.equal(err.message, exception, 'Wrong exception text'); | |||
There was a problem hiding this comment.
This was kind of a deliberate attempt to ensure that err always had a .message property (i.e., was an Error). Do you recall the scenario that required you to make a change here?
There was a problem hiding this comment.
I'm afraid I don't remember. I was getting an error (something like "undefined has no message property"), and it wasn't clear to me whether the error was coming from this line or from checkPages.js, so I decided to fix it here to reduce confusion for future me. If err doesn't have a message property, this test will still fail.
|
I really dug in to this bug, investigating with Wireshark. Firefox always URI encodes URLs before putting them in the GET HTTP header. However, it seems that Node's This is bug #2114 in Node.js and I want to see if my pull request gets accepted. |
|
Thanks for the update! Now I'm curious as well. :) |
Take a web page, which is correctly encoded in UTF-8 and has set the UTF-8 header correctly, that has a link like this:
If you click on the link, the browser encodes/escapes the URL to this:
And sends a GET request with that URL. If the browser forgets to do encode/escape it, some servers won't accept the request (although it seems some servers do recover well).
The patch included fixes check-pages to handle this use-case correctly, even when the URL contains white-space, and it includes a test.