Change the parseQueryString function to return a Map rather than an Object (issue 13829)
#13831
Conversation
…an Object (issue 13829)
Even though the code as-is *should* be safe, given that we're using an Object with a `null` prototype, it cannot hurt to change this to a Map to prevent any issues (since we're parsing unknown and potentially unsafe data).
Overall I also think that these changes improve the `parseQueryString` call-sites, since we now have a proper way of checking for the existence of a particular key (and don't have to use `in` which stringifies the keys in the Object).
This patch also changes the default, when no `value` exists, from `null` to an empty string since the use of `decodeURIComponent` currently can modify the value in a somewhat surprising way (at least to me).
Note how `decodeURIComponent(null) === "null"` which is unlikely to be what you actually want, whereas `decodeURIComponent("") === ""` which seems much more helpful.
|
/botio unittest |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/c28512f63c2e27b/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/4d541c96b3b1363/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/4d541c96b3b1363/output.txt Total script time: 3.93 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/c28512f63c2e27b/output.txt Total script time: 5.56 mins
|
|
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/192a9fb4e95ded9/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/f640970299a0d43/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/192a9fb4e95ded9/output.txt Total script time: 4.31 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/f640970299a0d43/output.txt Total script time: 5.19 mins
|
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/4b7960dc480d629/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/4b7960dc480d629/output.txt Total script time: 5.56 mins Published |
|
Looks better and fixes the alert, so let's do this; thanks! |
Even though the code as-is should be safe, given that we're using an Object with a
nullprototype, it cannot hurt to change this to a Map to prevent any issues (since we're parsing unknown and potentially unsafe data).Overall I also think that these changes improve the
parseQueryStringcall-sites, since we now have a proper way of checking for the existence of a particular key (and don't have to useinwhich stringifies the keys in the Object).This patch also changes the default, when no
valueexists, fromnullto an empty string since the use ofdecodeURIComponentcurrently can modify the value in a somewhat surprising way (at least to me).Note how
decodeURIComponent(null) === "null"which is unlikely to be what you actually want, whereasdecodeURIComponent("") === ""which seems much more helpful.Edit: https://github.com/mozilla/pdf.js/pull/13831/checks?check_run_id=3210760893