sindresorhus / got

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

Spaces should be normalized as `+` in query strings #1113

Closed Richienb closed 4 years ago

Richienb commented 4 years ago

What problem are you trying to solve?

Some web services like the VLC Web Interface use %20 to represent spaces in search params. However, Got automatically forces + with no way to override it even if put through query-string.

Describe the feature

The simplest solution would be to allow such overrides using the current syntax:

const got = require("got")
const queryString = require("query-string")

await got(`http://192.168.1.2/requests/status.json`), {
    port,
    password,
    searchParams: queryString.stringify({
        command,
        ...options,
    }).replace(/\+/, "%20")
})

The current workaround is to insert it directly into the url:

const got = require("got")
const queryString = require("query-string")

await got(`http://192.168.1.2/requests/status.json?${queryString.stringify({
    command,
    ...options,
}).replace(/\+/, "%20")}`), {
    port,
    password,
})

See: https://github.com/Richienb/vlc/blob/62f395cd471cf9cad0c540bad3aade8d2a7faa6f/src/index.ts#L169-L175

Checklist

szmarczak commented 4 years ago

I cannot reproduce, can you create a RunKit example?

Richienb commented 4 years ago

@szmarczak Copy and paste this into npm.runkit.com:

const got = require("got")
const queryString = require("query-string")

console.log((await got(`http://example.com`, {
    searchParams: queryString.stringify({
        a: "b c"
    }).replace(/\+/, "%20")
})).url)

It is supposed to log a URL with %20 as space.

szmarczak commented 4 years ago

You're using the searchparams option which converts the provided into a URLSearchParams instance. It converts %20 into +.

https://runkit.com/szmarczak/5e6aa4e35cec7b001b102009

szmarczak commented 4 years ago

Reference: https://url.spec.whatwg.org/#urlencoded-parsing

szmarczak commented 4 years ago

@sindresorhus This is inconsistent:

new URL('https://example.com/?a=b c').toString()
"https://example.com/?a=b%20c"

new URLSearchParams('a=b c').toString()
"a=b+c"
szmarczak commented 4 years ago
const url = new URL('https://example.com/?a=b c');

url.search
"?a=b%20c"
url.searchParams.toString()
"a=b+c"
szmarczak commented 4 years ago

The WHATWG URL says that:

A URL object has an associated url (a URL) and query object (a URLSearchParams object).

szmarczak commented 4 years ago

But it also says:

A URL’s query is either null or an ASCII string holding data. It is initially null.

szmarczak commented 4 years ago

url.searchParams.set('a', url.searchParams.get('a')) converts ?a=b%20c into ?a=b+c

szmarczak commented 4 years ago

Some web services like the VLC Web Interface use %20 to represent spaces in search params.

You should make an issue there instead. WHATWG URL is a standard and + should be accepted.

szmarczak commented 4 years ago

Modifying url.searchParams in any way (sort/append/delete) normalizes the query string properly.

@sindresorhus Maybe let's do url.searchParams.sort()?

sindresorhus commented 4 years ago

Modifying url.searchParams in any way (sort/append/delete) normalizes the query string properly.

Since this works, it sounds like the original issue is more likely a Node.js bug in the URL implementation?

Maybe let's do url.searchParams.sort()?

Some people unfortunately depend on the order. I experienced that with https://github.com/sindresorhus/query-string

However, maybe we can just add a random obscure value and remove it again. Does that cause it to normalize? Like __GOT__INTERNAL__THING__.

szmarczak commented 4 years ago

Since this works, it sounds like the original issue is more likely a Node.js bug in the URL implementation?

It's in V8.

However, maybe we can just add a random obscure value and remove it again. Does that cause it to normalize? Like GOTINTERNALTHING.

image

sindresorhus commented 4 years ago

It's in V8.

How is it a bug in V8?


I would prefer workaround number 3.

szmarczak commented 4 years ago

How is it a bug in V8?

The bug also appears in browsers...

sindresorhus commented 4 years ago

@szmarczak That doesn't necessarily mean it's a bug in V8 though. Both the browser and Node.js URL implementation is based on the same spec and might have the same implementation bug or spec bug.

TimothyGu commented 4 years ago

There's a spec bug for the inconsistency between search and URLSearchParams: https://github.com/whatwg/url/issues/18#issuecomment-269649720

The implementation for URL and URLSearchParams is not in V8. Node.js and Chromium have separate implementations, with Node.js much closer to the WHATWG URL Standard than Chromium.

tusbar commented 4 years ago

I just observed the same issue – it’s not uncommon for web servers to not accept +: Amazon’s MWS API only supports %20 in form body and query string.

Workaround’s the same as the OP: cannot use searchParams nor form options.

szmarczak commented 4 years ago

Amazon’s MWS API only supports %20 in form body and query string

We do everything according to the spec, if they don't follow it - email them.