Deno: URLSearchParams.toString() differs from browser output for spaces

Created on 10 Aug 2020  路  9Comments  路  Source: denoland/deno

Deno seems to deal with spaces differently to browsers when it comes to URLSearchParams, where a browser will replace spaces with a +, deno will insert %20 instead.

new URLSearchParams({a: 'hello world'}).toString()
// result in chrome/firefox:  "a=hello+world"
// result in deno: `a=hello%20world

URLSearchParams are supposed to encode in application/x-www-form-urlencoded format, which would have spaces become + like chrome/firefox are doing.


Depending on the context, the character ' ' is translated to a '+' (like in the percent-encoding version used in an application/x-www-form-urlencoded message)

https://developer.mozilla.org/en-US/docs/Glossary/percent-encoding

URLSearchParams objects will percent-encode anything in the application/x-www-form-urlencoded percent-encode set, and will encode U+0020 SPACE as U+002B (+).

https://url.spec.whatwg.org/#example-constructing-urlsearchparams


Versions

deno 1.2.3
v8 8.6.334
typescript 3.9.2

bug good first issue web

Most helpful comment

At the bottom or 11_url.js, you'll see a bunch of functions encodePathname(), encodeHash(), etc. Add another one there for this purpose implemented in a similar way. No it should not refer to anything external to cli/rt.

All 9 comments

Marking this as a good first issue for anyone wanting to explore how our internal runtime code looks. Fix is somewhere here:
https://github.com/denoland/deno/blob/67fe8cd8484611a1cbd72d058539920d09b18cfc/cli/rt/11_url.js
test should be added
https://github.com/denoland/deno/blob/67fe8cd8484611a1cbd72d058539920d09b18cfc/cli/tests/unit/url_search_params_test.ts

Dibs! 馃榿

@JayHelton mind if I pick this one? it looks an easy fix for someone with low experience with deno like me :)

@JayHelton mind if I pick this one? it looks an easy fix for someone with low experience with deno like me :)

Yeah go for it! I have the fix "planned out", so to speak. I did do the prior workings research against nodejs. So the actually logic change is pretty straightforward.

Deno should have all of the other dependencies needed (encodeStrin the querystring module).

Let me know if you want a hand or want to pair at all! 馃榿 @Sparkenstein

Deno should have all of the other dependencies needed (encodeStrin the querystring module).

Am I correct in thinking that this means it's okay for Deno to depend on std/node? Does this happen elsewhere in Deno as well? Or did you mean he should yoink the implementation?

Deno should have all of the other dependencies needed (encodeStrin the querystring module).

Am I correct in thinking that this means it's okay for Deno to depend on std/node? Does this happen elsewhere in Deno as well? Or did you mean he should yoink the implementation?

Ill call on @ry and @nayeemrmn for this.

Deno has an encodeStr method defined in deno/std/node/querystring.ts. Im not sure the design decisions made or preferences around actually using that, but I know we will probably need _something_ like it. encodeURIComponent doesnt fit the application/x-www-form-urlencoded standards

actually reading the readme, i see **Warning**: Any function of this module should not be referred anywhere in the deno standard library as it's a compatibility module.

So we dont want to use this in the std lib, so probably need to have a deno impl?

At the bottom or 11_url.js, you'll see a bunch of functions encodePathname(), encodeHash(), etc. Add another one there for this purpose implemented in a similar way. No it should not refer to anything external to cli/rt.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

JosephAkayesi picture JosephAkayesi  路  3Comments

benjamingr picture benjamingr  路  3Comments

ry picture ry  路  3Comments

justjavac picture justjavac  路  3Comments

somombo picture somombo  路  3Comments