events: refactor to use primordials in lib/events by marsonya · Pull Request #38117 · nodejs/node · GitHub

Replace code that's vulnerable to Prototype Pollution with Primordials.

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/993/

Benchmarks should be checked on this before landing

Some significant perf regressions:

                                                         confidence improvement accuracy (*)   (**)  (***)
events/ee-add-remove.jsn=1000000                                ***    -27.64 %       ±3.09% ±4.13% ±5.40%
events/ee-emit.jslisteners=10 argc=0 n=2000000                           0.28 %       ±1.22% ±1.63% ±2.12%
events/ee-emit.jslisteners=10 argc=10 n=2000000                          0.44 %       ±1.47% ±1.96% ±2.58%
events/ee-emit.jslisteners=10 argc=2 n=2000000                           0.37 %       ±1.27% ±1.69% ±2.20%
events/ee-emit.jslisteners=10 argc=4 n=2000000                           0.04 %       ±1.07% ±1.43% ±1.86%
events/ee-emit.jslisteners=1 argc=0 n=2000000                           -2.06 %       ±2.67% ±3.55% ±4.62%
events/ee-emit.jslisteners=1 argc=10 n=2000000                          -1.66 %       ±3.41% ±4.54% ±5.91%
events/ee-emit.jslisteners=1 argc=2 n=2000000                           -4.38 %       ±4.59% ±6.11% ±7.96%
events/ee-emit.jslisteners=1 argc=4 n=2000000                           -1.97 %       ±3.47% ±4.63% ±6.03%
events/ee-emit.jslisteners=5 argc=0 n=2000000                           -0.08 %       ±2.29% ±3.04% ±3.96%
events/ee-emit.jslisteners=5 argc=10 n=2000000                          -0.77 %       ±1.88% ±2.51% ±3.26%
events/ee-emit.jslisteners=5 argc=2 n=2000000                           -0.76 %       ±1.97% ±2.63% ±3.43%
events/ee-emit.jslisteners=5 argc=4 n=2000000                           -0.21 %       ±2.09% ±2.78% ±3.62%
events/ee-listener-count-on-prototype.jsn=50000000                *     -5.06 %       ±4.62% ±6.21% ±8.20%
events/ee-listeners.jsraw='false' listeners=50 n=5000000                 1.91 %       ±3.18% ±4.26% ±5.58%
events/ee-listeners.jsraw='false' listeners=5 n=5000000                 -1.18 %       ±3.36% ±4.50% ±5.91%
events/ee-listeners.jsraw='true' listeners=50 n=5000000                  1.67 %       ±2.85% ±3.80% ±4.98%
events/ee-listeners.jsraw='true' listeners=5 n=5000000                   2.24 %       ±3.54% ±4.71% ±6.14%
events/ee-once.jsargc=0 n=20000000                              ***     -6.95 %       ±1.40% ±1.86% ±2.43%
events/ee-once.jsargc=1 n=20000000                              ***     -9.55 %       ±1.17% ±1.56% ±2.03%
events/ee-once.jsargc=4 n=20000000                              ***     -9.36 %       ±4.11% ±5.47% ±7.12%
events/ee-once.jsargc=5 n=20000000                              ***     -8.47 %       ±2.74% ±3.64% ±4.74%
events/eventtarget.jslisteners=10 n=1000000                       *      3.56 %       ±3.01% ±4.02% ±5.28%
events/eventtarget.jslisteners=1 n=1000000                              -3.34 %       ±4.28% ±5.74% ±7.54%
events/eventtarget.jslisteners=5 n=1000000                               0.94 %       ±2.38% ±3.16% ±4.12%

I'm -1 to land this with regressions.

Need the perf regressions figured out

These are the primordials causing the Regressions in my previous commit

ArrayPrototypeUnshift and ArrayPrototypePush in _addListener()
FunctionPrototypeCall and ReflectApply in onceWrapper()
FunctionPrototypeBind in _onceWrap()
ArrayPrototypeShift in removeListener()
ArrayPrototypeShift in on()

I have removed these and benchmarked events again (locally on my computer).
The regressions are gone.

Kindly re-run benchmarks once again.

These are the primordials causing the Regressions in my previous commit

ArrayPrototypeUnshift and ArrayPrototypePush in _addListener()
FunctionPrototypeCall and ReflectApply in onceWrapper()
FunctionPrototypeBind in _onceWrap()
ArrayPrototypeShift in removeListener()
ArrayPrototypeShift in on()

I have removed these and benchmarked events again (locally on my computer).
The regressions are gone.

Kindly re-run benchmarks once again.

Sorry for leaving a review instead of comment.
I tried deleting it and reposting it. Couldn't delete the review.

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1010/

There does not seem to be any significant performance regressions in the bechmark run.

Details

14:30:14                                                           confidence improvement accuracy (*)    (**)   (***)
14:30:15 events/ee-add-remove.js n=1000000                                        -1.98 %       ±5.20%  ±6.92%  ±9.01%
14:30:15 events/ee-emit.js listeners=10 argc=0 n=2000000                           0.07 %       ±1.19%  ±1.58%  ±2.06%
14:30:15 events/ee-emit.js listeners=10 argc=10 n=2000000                          1.95 %       ±3.77%  ±5.05%  ±6.64%
14:30:15 events/ee-emit.js listeners=10 argc=2 n=2000000                           0.08 %       ±0.81%  ±1.08%  ±1.40%
14:30:15 events/ee-emit.js listeners=10 argc=4 n=2000000                           0.40 %       ±0.98%  ±1.30%  ±1.69%
14:30:15 events/ee-emit.js listeners=1 argc=0 n=2000000                            1.63 %       ±4.24%  ±5.64%  ±7.34%
14:30:15 events/ee-emit.js listeners=1 argc=10 n=2000000                          -0.60 %       ±3.49%  ±4.64%  ±6.04%
14:30:15 events/ee-emit.js listeners=1 argc=2 n=2000000                           -1.54 %       ±4.00%  ±5.32%  ±6.93%
14:30:15 events/ee-emit.js listeners=1 argc=4 n=2000000                           -5.19 %       ±7.62% ±10.23% ±13.53%
14:30:15 events/ee-emit.js listeners=5 argc=0 n=2000000                           -1.80 %       ±2.15%  ±2.86%  ±3.73%
14:30:15 events/ee-emit.js listeners=5 argc=10 n=2000000                          -0.44 %       ±2.13%  ±2.83%  ±3.69%
14:30:15 events/ee-emit.js listeners=5 argc=2 n=2000000                            0.56 %       ±2.15%  ±2.86%  ±3.73%
14:30:15 events/ee-emit.js listeners=5 argc=4 n=2000000                           -1.72 %       ±2.54%  ±3.38%  ±4.40%
14:30:15 events/ee-listener-count-on-prototype.js n=50000000                       0.19 %       ±5.10%  ±6.79%  ±8.85%
14:30:15 events/ee-listeners.js raw='false' listeners=50 n=5000000                 0.57 %       ±1.29%  ±1.72%  ±2.23%
14:30:15 events/ee-listeners.js raw='false' listeners=5 n=5000000                  0.65 %       ±1.37%  ±1.82%  ±2.37%
14:30:15 events/ee-listeners.js raw='true' listeners=50 n=5000000                 -2.61 %       ±3.80%  ±5.10%  ±6.73%
14:30:15 events/ee-listeners.js raw='true' listeners=5 n=5000000                  -1.70 %       ±5.65%  ±7.58%  ±9.99%
14:30:15 events/ee-once.js argc=0 n=20000000                                       1.25 %       ±1.45%  ±1.94%  ±2.54%
14:30:15 events/ee-once.js argc=1 n=20000000                                      -1.49 %       ±2.15%  ±2.88%  ±3.79%
14:30:15 events/ee-once.js argc=4 n=20000000                                      -0.29 %       ±1.31%  ±1.75%  ±2.28%
14:30:15 events/ee-once.js argc=5 n=20000000                                       0.09 %       ±1.34%  ±1.79%  ±2.32%
14:30:15 events/eventtarget.js listeners=10 n=1000000                              0.94 %       ±2.36%  ±3.15%  ±4.10%
14:30:15 events/eventtarget.js listeners=1 n=1000000                               0.07 %       ±1.75%  ±2.34%  ±3.05%
14:30:15 events/eventtarget.js listeners=5 n=1000000                        *      7.80 %       ±6.65%  ±8.95% ±11.85%

CI: https://ci.nodejs.org/job/node-test-pull-request/37534/

Benchmark results look promising! It's probably a good idea to wait for #38248 to land first as it's going to create conflicts and we want to include it in the v16.0.0 release.

Once this has been rebased on top of #38248 (after it has landed), we should also run HTTP benchmark to make sure we're not introducing hidden perf regressions.

Benchmark results look promising! It's probably a good idea to wait for #38248 to land first as it's going to create conflicts and we want to include it in the v16.0.0 release.

#38248 has landed. can we proceed with this change?

We need to validate that http did not regress.

@marsonya if you want to rebase on top of master to fix the git conflict, we can spawn some benchmarks to see where this PR stands in term of perf regressions/improvment.

Replace code that's vulnerable to Prototype Pollution with Primordials.

Benchmark CI (events): https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1019/
Benchmark CI (http/client-request-body): https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1020/ (queued)
Benchmark CI (http/end-vs-write-end): https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1026/ (queued)
Benchmark CI (http/*header*): https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1022/ (queued)
Benchmark CI (http/upgrade): https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1023/ (queued)
Benchmark CI (http/check*): https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1024/ (queued)
Benchmark CI (http/simple): https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1025/ (failed)

Results

                                                                 confidence improvement accuracy (*)   (**)   (***)
events/ee-add-remove.js n=1000000 removeListener=0 newListener=0                 1.41 %       ±2.72% ±3.65%  ±4.81%
events/ee-add-remove.js n=1000000 removeListener=0 newListener=1                -0.75 %       ±2.15% ±2.87%  ±3.75%
events/ee-add-remove.js n=1000000 removeListener=1 newListener=0                -0.23 %       ±2.13% ±2.84%  ±3.70%
events/ee-add-remove.js n=1000000 removeListener=1 newListener=1                 1.49 %       ±3.19% ±4.25%  ±5.55%
events/ee-emit.js listeners=10 argc=0 n=2000000                                 -0.77 %       ±1.34% ±1.79%  ±2.32%
events/ee-emit.js listeners=10 argc=10 n=2000000                                -1.11 %       ±2.08% ±2.78%  ±3.66%
events/ee-emit.js listeners=10 argc=2 n=2000000                                 -1.29 %       ±2.52% ±3.39%  ±4.47%
events/ee-emit.js listeners=10 argc=4 n=2000000                                 -0.31 %       ±1.04% ±1.38%  ±1.79%
events/ee-emit.js listeners=1 argc=0 n=2000000                                  -1.38 %       ±3.99% ±5.33%  ±6.97%
events/ee-emit.js listeners=1 argc=10 n=2000000                                  1.47 %       ±5.93% ±7.93% ±10.39%
events/ee-emit.js listeners=1 argc=2 n=2000000                                  -1.31 %       ±4.75% ±6.34%  ±8.30%
events/ee-emit.js listeners=1 argc=4 n=2000000                                  -3.11 %       ±3.65% ±4.86%  ±6.33%
events/ee-emit.js listeners=5 argc=0 n=2000000                                   0.87 %       ±2.23% ±2.96%  ±3.86%
events/ee-emit.js listeners=5 argc=10 n=2000000                                  1.65 %       ±2.46% ±3.28%  ±4.27%
events/ee-emit.js listeners=5 argc=2 n=2000000                                   0.64 %       ±1.89% ±2.52%  ±3.27%
events/ee-emit.js listeners=5 argc=4 n=2000000                                  -0.06 %       ±1.99% ±2.66%  ±3.46%
events/ee-listener-count-on-prototype.js n=50000000                              1.08 %       ±3.71% ±4.94%  ±6.43%
events/ee-listeners.js raw='false' listeners=50 n=5000000                        0.59 %       ±2.46% ±3.28%  ±4.30%
events/ee-listeners.js raw='false' listeners=5 n=5000000                        -0.29 %       ±3.00% ±4.01%  ±5.26%
events/ee-listeners.js raw='true' listeners=50 n=5000000                        -2.70 %       ±5.11% ±6.86%  ±9.07%
events/ee-listeners.js raw='true' listeners=5 n=5000000                          3.39 %       ±4.46% ±5.94%  ±7.76%
events/ee-once.js argc=0 n=20000000                                              0.04 %       ±1.24% ±1.67%  ±2.20%
events/ee-once.js argc=1 n=20000000                                             -0.10 %       ±4.28% ±5.69%  ±7.41%
events/ee-once.js argc=4 n=20000000                                              0.38 %       ±1.53% ±2.06%  ±2.71%
events/ee-once.js argc=5 n=20000000                                              0.51 %       ±1.26% ±1.68%  ±2.21%
events/eventtarget.js listeners=10 n=1000000                                     1.54 %       ±7.18% ±9.55% ±12.43%
events/eventtarget.js listeners=1 n=1000000                                      3.55 %       ±6.82% ±9.08% ±11.82%
events/eventtarget.js listeners=5 n=1000000                                     -2.66 %       ±5.95% ±7.92% ±10.33%
                                                                     confidence improvement accuracy (*)    (**)   (***)
http/client-request-body.js method='end' len=1024 type='asc' dur=5                  -2.08 %       ±6.24%  ±8.30% ±10.80%
http/client-request-body.js method='end' len=1024 type='buf' dur=5                  -2.93 %       ±7.54% ±10.04% ±13.06%
http/client-request-body.js method='end' len=1024 type='utf' dur=5                  -2.02 %       ±6.33%  ±8.42% ±10.96%
http/client-request-body.js method='end' len=256 type='asc' dur=5                    0.76 %       ±7.37%  ±9.81% ±12.76%
http/client-request-body.js method='end' len=256 type='buf' dur=5                    0.62 %       ±6.90%  ±9.19% ±11.96%
http/client-request-body.js method='end' len=256 type='utf' dur=5                    4.99 %       ±7.03%  ±9.35% ±12.17%
http/client-request-body.js method='end' len=32 type='asc' dur=5                    -0.69 %       ±7.30%  ±9.72% ±12.65%
http/client-request-body.js method='end' len=32 type='buf' dur=5                     0.59 %       ±7.22%  ±9.61% ±12.51%
http/client-request-body.js method='end' len=32 type='utf' dur=5                     3.04 %       ±6.21%  ±8.27% ±10.76%
http/client-request-body.js method='write' len=1024 type='asc' dur=5                 0.42 %       ±6.92%  ±9.21% ±12.00%
http/client-request-body.js method='write' len=1024 type='buf' dur=5                 1.79 %       ±6.19%  ±8.24% ±10.73%
http/client-request-body.js method='write' len=1024 type='utf' dur=5                -4.16 %       ±5.91%  ±7.87% ±10.24%
http/client-request-body.js method='write' len=256 type='asc' dur=5                 -4.80 %       ±5.32%  ±7.08%  ±9.22%
http/client-request-body.js method='write' len=256 type='buf' dur=5                 -3.92 %       ±6.94%  ±9.23% ±12.02%
http/client-request-body.js method='write' len=256 type='utf' dur=5                 -0.06 %       ±6.99%  ±9.30% ±12.11%
http/client-request-body.js method='write' len=32 type='asc' dur=5                  -0.17 %       ±6.23%  ±8.29% ±10.79%
http/client-request-body.js method='write' len=32 type='buf' dur=5                  -3.22 %       ±5.71%  ±7.60%  ±9.89%
http/client-request-body.js method='write' len=32 type='utf' dur=5                   2.09 %       ±6.88%  ±9.17% ±11.95%
                                                                                              confidence improvement accuracy (*)   (**)   (***)
                                                                                                      NA       NaN %           NA     NA      NA
http/check_invalid_header_char.js n=1000000 input=''                                                         -0.43 %       ±6.31% ±8.40% ±10.93%
http/check_invalid_header_char.js n=1000000 input='1'                                                         1.42 %       ±4.90% ±6.52%  ±8.49%
http/check_invalid_header_char.js n=1000000 input='20091'                                                     2.33 %       ±4.36% ±5.81%  ±7.57%
http/check_invalid_header_char.js n=1000000 input='close'                                                     0.20 %       ±4.35% ±5.79%  ±7.54%
http/check_invalid_header_char.js n=1000000 input='en-US'                                                     0.27 %       ±3.14% ±4.17%  ±5.43%
http/check_invalid_header_char.js n=1000000 input='foo\\nbar'                                                -4.35 %       ±5.03% ±6.71%  ±8.76%
http/check_invalid_header_char.js n=1000000 input='group_acmeair'                                            -2.05 %       ±3.49% ±4.65%  ±6.06%
http/check_invalid_header_char.js n=1000000 input='gzip'                                                      0.23 %       ±4.54% ±6.04%  ±7.86%
http/check_invalid_header_char.js n=1000000 input='keep-alive'                                               -1.70 %       ±4.36% ±5.80%  ±7.55%
http/check_invalid_header_char.js n=1000000 input='LONG_AND_INVALID'                                         -0.43 %       ±2.02% ±2.68%  ±3.49%
http/check_invalid_header_char.js n=1000000 input='private'                                                   1.65 %       ±4.70% ±6.27%  ±8.20%
http/check_invalid_header_char.js n=1000000 input='SAMEORIGIN'                                                2.27 %       ±4.58% ±6.11%  ±7.96%
http/check_invalid_header_char.js n=1000000 input='Sat, 07 May 2016 16:54:48 GMT'                             1.14 %       ±3.96% ±5.28%  ±6.87%
http/check_invalid_header_char.js n=1000000 input='text/html; charset=utf-8'                                 -1.90 %       ±4.41% ±5.88%  ±7.65%
http/check_invalid_header_char.js n=1000000 input='text/plain'                                                0.33 %       ±4.28% ±5.70%  ±7.44%
http/check_invalid_header_char.js n=1000000 input='\\t\\t\\t\\t\\t\\t\\t\\t\\t\\tFoo bar baz'                -0.80 %       ±3.32% ±4.42%  ±5.77%
http/check_invalid_header_char.js n=1000000 input='\\x7F'                                                     0.55 %       ±4.73% ±6.30%  ±8.20%
http/check_invalid_header_char.js n=1000000 input='中文呢'                                                   -0.24 %       ±4.16% ±5.53%  ±7.21%
http/headers.js duration=5 len=100 n=10 benchmarker='wrk'                                                     1.35 %       ±3.45% ±4.59%  ±5.98%
http/headers.js duration=5 len=100 n=600 benchmarker='wrk'                                                   -0.24 %       ±4.83% ±6.43%  ±8.38%
http/headers.js duration=5 len=1 n=10 benchmarker='wrk'                                                      -1.59 %       ±5.20% ±6.91%  ±9.00%
http/headers.js duration=5 len=1 n=600 benchmarker='wrk'                                                      0.00 %       ±3.23% ±4.31%  ±5.62%
http/incoming_headers.js duration=5 w=0 headers=20 connections=50 benchmarker='wrk'                           4.71 %       ±4.90% ±6.52%  ±8.49%
http/incoming_headers.js duration=5 w=6 headers=20 connections=50 benchmarker='wrk'                          -1.63 %       ±4.99% ±6.65%  ±8.67%
http/set-header.js duration=5 res='normal' benchmarker='wrk'                                                 -1.62 %       ±4.20% ±5.59%  ±7.28%
http/set-header.js duration=5 res='setHeader' benchmarker='wrk'                                              -2.73 %       ±3.81% ±5.07%  ±6.59%
http/set-header.js duration=5 res='setHeaderWH' benchmarker='wrk'                                             1.61 %       ±4.58% ±6.09%  ±7.93%
http/set_header.js n=1000000 value='Connection'                                                               0.83 %       ±2.04% ±2.72%  ±3.54%
http/set_header.js n=1000000 value='Content-Length'                                                           0.71 %       ±2.23% ±2.99%  ±3.92%
http/set_header.js n=1000000 value='Content-Type'                                                            -0.42 %       ±1.78% ±2.37%  ±3.09%
http/set_header.js n=1000000 value='Set-Cookie'                                                               0.39 %       ±1.76% ±2.34%  ±3.05%
http/set_header.js n=1000000 value='Transfer-Encoding'                                                        1.97 %       ±3.40% ±4.57%  ±6.05%
http/set_header.js n=1000000 value='Vary'                                                                     1.49 %       ±1.95% ±2.59%  ±3.37%
http/set_header.js n=1000000 value='X-Powered-By'                                                             1.21 %       ±1.82% ±2.42%  ±3.15%
                       confidence improvement accuracy (*)   (**)   (***)
http/upgrade.js n=1000                 2.07 %       ±5.89% ±7.84% ±10.21%
http/upgrade.js n=5                   -0.38 %       ±5.23% ±6.96%  ±9.05%
                                                                                              confidence improvement accuracy (*)   (**)   (***)
http/check_invalid_header_char.js n=1000000 input=''                                                         -3.32 %       ±3.63% ±4.83%  ±6.28%
http/check_invalid_header_char.js n=1000000 input='1'                                                         0.12 %       ±3.58% ±4.76%  ±6.20%
http/check_invalid_header_char.js n=1000000 input='20091'                                                     1.52 %       ±5.04% ±6.70%  ±8.73%
http/check_invalid_header_char.js n=1000000 input='close'                                                     1.90 %       ±4.51% ±6.01%  ±7.84%
http/check_invalid_header_char.js n=1000000 input='en-US'                                                    -1.46 %       ±3.23% ±4.30%  ±5.59%
http/check_invalid_header_char.js n=1000000 input='foo\\nbar'                                                -3.71 %       ±4.28% ±5.70%  ±7.43%
http/check_invalid_header_char.js n=1000000 input='group_acmeair'                                      *     -3.12 %       ±3.06% ±4.09%  ±5.34%
http/check_invalid_header_char.js n=1000000 input='gzip'                                                     -2.64 %       ±5.31% ±7.07%  ±9.22%
http/check_invalid_header_char.js n=1000000 input='keep-alive'                                                0.61 %       ±5.27% ±7.01%  ±9.13%
http/check_invalid_header_char.js n=1000000 input='LONG_AND_INVALID'                                          0.18 %       ±2.27% ±3.02%  ±3.94%
http/check_invalid_header_char.js n=1000000 input='private'                                                   0.41 %       ±2.83% ±3.77%  ±4.91%
http/check_invalid_header_char.js n=1000000 input='SAMEORIGIN'                                               -1.72 %       ±3.89% ±5.17%  ±6.74%
http/check_invalid_header_char.js n=1000000 input='Sat, 07 May 2016 16:54:48 GMT'                             0.01 %       ±2.57% ±3.42%  ±4.46%
http/check_invalid_header_char.js n=1000000 input='text/html; charset=utf-8'                                  0.90 %       ±6.15% ±8.19% ±10.66%
http/check_invalid_header_char.js n=1000000 input='text/plain'                                                3.01 %       ±3.83% ±5.10%  ±6.66%
http/check_invalid_header_char.js n=1000000 input='\\t\\t\\t\\t\\t\\t\\t\\t\\t\\tFoo bar baz'                -0.11 %       ±3.64% ±4.86%  ±6.36%
http/check_invalid_header_char.js n=1000000 input='\\x7F'                                                    -3.55 %       ±5.40% ±7.19%  ±9.36%
http/check_invalid_header_char.js n=1000000 input='中文呢'                                                    1.29 %       ±3.80% ±5.06%  ±6.59%
http/check_is_http_token.js n=1000000 key=':'                                                                -0.63 %       ±5.20% ±6.92%  ±9.01%
http/check_is_http_token.js n=1000000 key='((((())))'                                                  *      5.60 %       ±4.53% ±6.04%  ±7.87%
http/check_is_http_token.js n=1000000 key='@@'                                                                3.92 %       ±6.07% ±8.09% ±10.54%
http/check_is_http_token.js n=1000000 key='Accept-Ranges'                                                     0.97 %       ±3.76% ±5.01%  ±6.55%
http/check_is_http_token.js n=1000000 key=':alternate-protocol'                                               0.22 %       ±5.58% ±7.42%  ±9.65%
http/check_is_http_token.js n=1000000 key='alternate-protocol:'                                        *      3.57 %       ±3.28% ±4.37%  ±5.69%
http/check_is_http_token.js n=1000000 key='alternate-protocol'                                               -2.00 %       ±3.20% ±4.26%  ±5.56%
http/check_is_http_token.js n=1000000 key='alt-svc'                                                           2.40 %       ±3.75% ±4.99%  ±6.49%
http/check_is_http_token.js n=1000000 key='Cache-Control'                                                     1.64 %       ±4.13% ±5.50%  ±7.16%
http/check_is_http_token.js n=1000000 key='Connection'                                                       -2.00 %       ±4.67% ±6.22%  ±8.10%
http/check_is_http_token.js n=1000000 key='Content-Encoding'                                                 -2.40 %       ±4.07% ±5.42%  ±7.05%
http/check_is_http_token.js n=1000000 key='content-length'                                                    2.92 %       ±4.29% ±5.73%  ±7.48%
http/check_is_http_token.js n=1000000 key='Content-Location'                                                  2.00 %       ±3.92% ±5.21%  ±6.78%
http/check_is_http_token.js n=1000000 key='content-type'                                                     -0.14 %       ±3.99% ±5.31%  ±6.91%
http/check_is_http_token.js n=1000000 key='Content-Type'                                                      3.03 %       ±3.84% ±5.11%  ±6.65%
http/check_is_http_token.js n=1000000 key='date'                                                              1.07 %       ±3.70% ±4.92%  ±6.40%
http/check_is_http_token.js n=1000000 key='ETag'                                                             -1.31 %       ±4.70% ±6.26%  ±8.15%
http/check_is_http_token.js n=1000000 key='Expires'                                                          -2.46 %       ±6.07% ±8.11% ±10.63%
http/check_is_http_token.js n=1000000 key='Keep-Alive'                                                       -2.78 %       ±4.37% ±5.83%  ±7.62%
http/check_is_http_token.js n=1000000 key='Last-Modified'                                                     1.36 %       ±3.93% ±5.24%  ±6.82%
http/check_is_http_token.js n=1000000 key='location'                                                          2.02 %       ±3.01% ±4.00%  ±5.21%
http/check_is_http_token.js n=1000000 key='server'                                                            0.26 %       ±3.72% ±4.97%  ±6.50%
http/check_is_http_token.js n=1000000 key='Server'                                                            0.81 %       ±4.03% ±5.37%  ±7.00%
http/check_is_http_token.js n=1000000 key='status'                                                           -0.18 %       ±5.12% ±6.83%  ±8.91%
http/check_is_http_token.js n=1000000 key='TCN'                                                              -0.70 %       ±4.10% ±5.46%  ±7.11%
http/check_is_http_token.js n=1000000 key='Transfer-Encoding'                                                -0.06 %       ±4.74% ±6.31%  ±8.21%
http/check_is_http_token.js n=1000000 key='Vary'                                                             -2.94 %       ±4.37% ±5.81%  ±7.57%
http/check_is_http_token.js n=1000000 key='version'                                                          -1.39 %       ±4.36% ±5.81%  ±7.56%
http/check_is_http_token.js n=1000000 key='x-frame-options'                                                   3.44 %       ±4.60% ±6.14%  ±8.02%
http/check_is_http_token.js n=1000000 key='x-xss-protection'                                                 -0.28 %       ±4.10% ±5.45%  ±7.10%
http/check_is_http_token.js n=1000000 key='中文呢'                                                            1.51 %       ±4.26% ±5.67%  ±7.38%
                                                                                                  confidence improvement accuracy (*)    (**)   (***)
http/end-vs-write-end.js duration=5 method='end' c=100 len=1048576 type='asc' benchmarker='wrk'                  -0.48 %       ±2.77%  ±3.69%  ±4.80%
http/end-vs-write-end.js duration=5 method='end' c=100 len=1048576 type='buf' benchmarker='wrk'                   0.35 %       ±7.27%  ±9.68% ±12.61%
http/end-vs-write-end.js duration=5 method='end' c=100 len=1048576 type='utf' benchmarker='wrk'                  -1.16 %       ±2.05%  ±2.73%  ±3.55%
http/end-vs-write-end.js duration=5 method='end' c=100 len=131072 type='asc' benchmarker='wrk'                   -1.43 %       ±1.95%  ±2.60%  ±3.39%
http/end-vs-write-end.js duration=5 method='end' c=100 len=131072 type='buf' benchmarker='wrk'                   -0.57 %       ±3.76%  ±5.00%  ±6.51%
http/end-vs-write-end.js duration=5 method='end' c=100 len=131072 type='utf' benchmarker='wrk'                    2.05 %       ±2.14%  ±2.86%  ±3.77%
http/end-vs-write-end.js duration=5 method='end' c=100 len=262144 type='asc' benchmarker='wrk'                   -2.15 %       ±2.56%  ±3.42%  ±4.47%
http/end-vs-write-end.js duration=5 method='end' c=100 len=262144 type='buf' benchmarker='wrk'                   -2.64 %       ±5.75%  ±7.65%  ±9.95%
http/end-vs-write-end.js duration=5 method='end' c=100 len=262144 type='utf' benchmarker='wrk'                    0.49 %       ±3.59%  ±4.78%  ±6.23%
http/end-vs-write-end.js duration=5 method='end' c=100 len=65536 type='asc' benchmarker='wrk'                    -0.07 %       ±1.93%  ±2.57%  ±3.35%
http/end-vs-write-end.js duration=5 method='end' c=100 len=65536 type='buf' benchmarker='wrk'                     2.08 %       ±3.02%  ±4.03%  ±5.24%
http/end-vs-write-end.js duration=5 method='end' c=100 len=65536 type='utf' benchmarker='wrk'                    -2.18 %       ±3.61%  ±4.80%  ±6.24%
http/end-vs-write-end.js duration=5 method='write' c=100 len=1048576 type='asc' benchmarker='wrk'                -3.50 %       ±5.46%  ±7.26%  ±9.45%
http/end-vs-write-end.js duration=5 method='write' c=100 len=1048576 type='buf' benchmarker='wrk'                 0.55 %       ±7.65% ±10.18% ±13.25%
http/end-vs-write-end.js duration=5 method='write' c=100 len=1048576 type='utf' benchmarker='wrk'                 3.14 %       ±4.86%  ±6.47%  ±8.42%
http/end-vs-write-end.js duration=5 method='write' c=100 len=131072 type='asc' benchmarker='wrk'                 -2.09 %       ±5.47%  ±7.28%  ±9.49%
http/end-vs-write-end.js duration=5 method='write' c=100 len=131072 type='buf' benchmarker='wrk'                 -1.96 %       ±3.29%  ±4.38%  ±5.70%
http/end-vs-write-end.js duration=5 method='write' c=100 len=131072 type='utf' benchmarker='wrk'                 -1.37 %       ±5.23%  ±6.97%  ±9.09%
http/end-vs-write-end.js duration=5 method='write' c=100 len=262144 type='asc' benchmarker='wrk'                 -5.75 %       ±5.98%  ±7.95% ±10.35%
http/end-vs-write-end.js duration=5 method='write' c=100 len=262144 type='buf' benchmarker='wrk'                 -1.76 %       ±5.48%  ±7.29%  ±9.48%
http/end-vs-write-end.js duration=5 method='write' c=100 len=262144 type='utf' benchmarker='wrk'                 -1.00 %       ±5.19%  ±6.90%  ±8.99%
http/end-vs-write-end.js duration=5 method='write' c=100 len=65536 type='asc' benchmarker='wrk'                   1.56 %       ±5.34%  ±7.11%  ±9.25%
http/end-vs-write-end.js duration=5 method='write' c=100 len=65536 type='buf' benchmarker='wrk'                  -1.84 %       ±2.94%  ±3.91%  ±5.09%
http/end-vs-write-end.js duration=5 method='write' c=100 len=65536 type='utf' benchmarker='wrk'                   1.17 %       ±4.85%  ±6.46%  ±8.41%

Benchmark results look OK.

CI: https://ci.nodejs.org/job/node-test-pull-request/38227/

I would like to do some checks of my own before landing.

Temporary -1 until I can test this as well.

lgtm on my end

CI: https://ci.nodejs.org/job/node-test-pull-request/38241/ 💚

Landed in 13ec317

Replace code that's vulnerable to Prototype Pollution with Primordials. PR-URL: #38117 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

I'm going to mark this (and any other "use primordials" I come across) as requiring a manual backport due to the recent discussions around use of primordials in current and the fact that v14.x has an older version of V8 so if we did land these we would probably want to check benchmarks.

nodejs-github-bot added events Issues and PRs related to the events subsystem / EventEmitter. needs-ci PRs that need a full CI run. labels Apr 6, 2021

aduh95 reviewed Apr 6, 2021

View reviewed changes

Comment thread lib/events.js Outdated

jasnell previously approved these changes Apr 6, 2021

View reviewed changes

jasnell requested a review from mcollina April 6, 2021 21:30

aduh95 requested changes Apr 6, 2021

View reviewed changes

mcollina requested changes Apr 6, 2021

View reviewed changes

marsonya commented Apr 16, 2021

View reviewed changes

marsonya added the needs-benchmark-ci PR that need a benchmark CI run. label Apr 17, 2021

RaisinTen approved these changes Apr 17, 2021

View reviewed changes

RaisinTen requested review from aduh95 and mcollina April 17, 2021 13:41

aduh95 approved these changes Apr 17, 2021

View reviewed changes

aduh95 added the blocked PRs that are blocked by other issues or PRs. label Apr 17, 2021

events: refactor to use primordials in lib/events

d13628f

Replace code that's vulnerable to Prototype Pollution with Primordials.

aduh95 approved these changes May 20, 2021

View reviewed changes

aduh95 removed the blocked PRs that are blocked by other issues or PRs. label May 20, 2021

mcollina requested changes May 21, 2021

View reviewed changes

mcollina approved these changes May 21, 2021

View reviewed changes

aduh95 added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label May 21, 2021

jasnell approved these changes May 21, 2021

View reviewed changes

jasnell closed this May 21, 2021

danielleadams mentioned this pull request May 31, 2021

v16.3.0 proposal #38874

Merged

richardlau added the backport-requested-v14.x label Jul 16, 2021

events: refactor to use primordials in lib/events by marsonya · Pull Request #38117 · nodejs/node · GitHub

Uh oh!

Uh oh!

events: refactor to use primordials in lib/events - #38117

events: refactor to use primordials in lib/events#38117marsonya wants to merge 1 commit intonodejs:masternodejs/node:masterfrom marsonya:primordials-lib-eventsCopy head branch name to clipboard

Conversation

marsonya commented Apr 6, 2021

Uh oh!

Uh oh!

aduh95 commented Apr 6, 2021

Uh oh!

jasnell commented Apr 6, 2021

Uh oh!

aduh95 left a comment

Choose a reason for hiding this comment

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

marsonya left a comment

Choose a reason for hiding this comment

Uh oh!

marsonya commented Apr 16, 2021

Uh oh!

aduh95 commented Apr 17, 2021

Uh oh!

RaisinTen left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Apr 17, 2021

Uh oh!

aduh95 left a comment

Choose a reason for hiding this comment

Uh oh!

aduh95 commented Apr 17, 2021

Uh oh!

marsonya commented May 18, 2021

Uh oh!

mcollina commented May 18, 2021

Uh oh!

aduh95 commented May 18, 2021

Uh oh!

aduh95 commented May 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aduh95 left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented May 20, 2021

Uh oh!

mcollina commented May 21, 2021

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented May 21, 2021 • edited by jasnell Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jasnell commented May 21, 2021

Uh oh!

richardlau commented Jul 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

events: refactor to use primordials in lib/events#38117
marsonya wants to merge 1 commit into
nodejs:masterfrom
marsonya:primordials-lib-events

aduh95 commented May 19, 2021 •
edited
Loading

nodejs-github-bot commented May 21, 2021 •
edited by jasnell
Loading