sindresorhus / got

🌐 Human-friendly and powerful HTTP request library for Node.js
MIT License
14.27k stars 935 forks source link

Invalid extra escape for URLs #1180

Closed Hamper closed 4 years ago

Hamper commented 4 years ago

Describe the bug

Invalid extra escape for urls only in got v11, not in v10

Actual behavior

Got escapes some symbols like ~ in url but should not do this because some servers don't parse such escaped symbols

Expected behavior

Symbols in URLs like ~ must not be escaped

Code to reproduce

const got = require('got');
const testURL = 'https://example.com/?test=test~test';
console.log('RIGHT URL:\n%o\n', new URL(testURL));
(async ()=>{
    try{
        await got(testURL, {
            hooks: {
                beforeRequest: [
                    (options) => {
                        console.log('GOT URL:\n%o\n', options.url);
                    }
                ]
            }
        });
    }
    catch(e){
    }
})();

same result with new URL(testURL)) in got parameters

Checklist

szmarczak commented 4 years ago

It's a bug in the Node.js implementation. Try using URLSearchParams. The URL is normalized according to the spec. If servers don't follow the spec then it's not our problem.

Duplicate of #1113

Hamper commented 4 years ago

According to https://tools.ietf.org/html/rfc3986#section-2.3 tilde is unreserved and should not encoded by URI producers.

querystring.escape() - does not escape tilde new URL() - does not escape tilde encodeURIComponent() - does not escape tilde encodeURI() - does not escape tilde browsers - does not escape tilde, normalize %7E as ~ before sending got 10 - does not escape tilde

got 11 escapes tilde but should not according to RFC

szmarczak commented 4 years ago

Got doesn't do this. It's the native URLSearchParams. And, according to the WHATWG implementation the behavior is correct (see the original issue). I'll open a Node.js issue.