Skip to content

Is it valid not to throw error in querystring.encode? #3702

Closed
@hakatashi

Description

@hakatashi

Through reading source code for querystring.escape, I found that querystring.escape doesn't throw any error since #847 merge, even when invalid surrogate pairs are supplied.

Real world example:

vagrant@vagrant-ubuntu-trusty-64:~$ nvm use 0.12
Now using node v0.12.7 (npm v2.11.3)
vagrant@vagrant-ubuntu-trusty-64:~$ node
> var qs = require('querystring')
undefined
> qs.stringify({foo: '\udc00'})
URIError: URI malformed
    at encodeURIComponent (native)
    at QueryString.escape (querystring.js:118:10)
    at Object.QueryString.stringify.QueryString.encode (querystring.js:154:26)
    at repl:1:4
    at REPLServer.defaultEval (repl.js:132:27)
    at bound (domain.js:254:14)
    at REPLServer.runBound [as eval] (domain.js:267:12)
    at REPLServer.<anonymous> (repl.js:279:12)
    at REPLServer.emit (events.js:107:17)
    at REPLServer.Interface._onLine (readline.js:214:10)
>
(^C again to quit)
>
vagrant@vagrant-ubuntu-trusty-64:~$ nvm use 5
Now using node v5.0.0 (npm v3.3.6)
vagrant@vagrant-ubuntu-trusty-64:~$ node
> const qs = require('querystring')
undefined
> qs.stringify({foo: '\udc00'})
'foo=%F0%90%80%80'

Since I couldn't find any docs or comments about the behavior, I wonder if this is an intentional change or a regression. In either case, it should be included in its test.

Metadata

Metadata

Assignees

No one assigned

    Labels

    querystringIssues and PRs related to the built-in querystring module.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions