ljharb / qs

A querystring parser with nesting support
BSD 3-Clause "New" or "Revised" License
8.5k stars 729 forks source link

[Robustness] `stringify`: avoid relying on a global `undefined` #427

Closed Connormiha closed 2 years ago

Connormiha commented 2 years ago

I just removed needless(for these cases) typeof for checking for undefined

update: Added checking for typeof in parser.

Connormiha commented 2 years ago

@ljharb

These are not needless at all. undefined can be redefined in every scope except the global scope in ES5, and in ES3 - which this package supports - it can be modified in the global scope otherwise.

Yes, but there are many places with same check. For example https://github.com/ljharb/qs/blob/master/lib/stringify.js#L155 or https://github.com/ljharb/qs/blob/master/lib/stringify.js#L195

ljharb commented 2 years ago

In that case, those should be changed to use typeof instead :-) want to update the PR to do that?

Connormiha commented 2 years ago

@ljharb typeof or void 0? Which one is preferable?

Connormiha commented 2 years ago

@ljharb updated.

ljharb commented 2 years ago

Definitely the typeof, because it has minimal runtime weight.

codecov[bot] commented 2 years ago

Codecov Report

Merging #427 (0a1d3e8) into master (408ff95) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #427   +/-   ##
=======================================
  Coverage   99.85%   99.85%           
=======================================
  Files           8        8           
  Lines        1408     1408           
  Branches      172      172           
=======================================
  Hits         1406     1406           
  Misses          2        2           
Impacted Files Coverage Δ
lib/stringify.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 408ff95...0a1d3e8. Read the comment docs.