Optimize stringify-cookie by ~90% for RFC-allowed values#269
Optimize stringify-cookie by ~90% for RFC-allowed values#269saripovdenis wants to merge 5 commits intojshttp:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #269 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 1 1
Lines 160 260 +100
Branches 69 116 +47
==========================================
+ Hits 160 260 +100 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@blakeembrey what do you think about this tradeoff? |
|
This looks pretty great! I think it's worth the trade-off. One question, should we just allow all RFC valid values through without encoding? It may differ slightly from |
Great idea! Some cases would have different behaviour stringifyCookie({ foo: "a=b" })
// before: foo=a%3Db
// after: foo=a=bBut overall looks like a great win from perf side! I pushed code accordingly |
|
Hi @blakeembrey Friendly ping~ |
|
There's an existing regex |
|
Also I'm planning to land this in a new major version to eliminate concerns around breaking changes. If you want this in the current version I can cherry pick your original commit, just let me know. |
| }); | ||
|
|
||
| bench("rfc cookie-octets", () => { | ||
| cookie.stringifyCookie({ foo: "a=b+c/d?x%20" }); |
There was a problem hiding this comment.
Actually, we'd 100% need to encode % to avoid ambiguity here. So I think the fast path is cookieValueRegexp minus %?
Otherwise the round trip won't be correct.
| /** | ||
| * RegExp to match RFC 6265 cookie-octet values that need no URL encoding. | ||
| */ | ||
| const cookieOctetRegExp = /^[!#$%&'()*+\-.\/0-9:<=>?@A-Z[\]\^_`a-z{|}~]*$/; |
There was a problem hiding this comment.
| const cookieOctetRegExp = /^[!#$%&'()*+\-.\/0-9:<=>?@A-Z[\]\^_`a-z{|}~]*$/; | |
| const cookieOctetRegExp = /^[!#$&'()*+\-.\/0-9:<=>?@A-Z[\]\^_`a-z{|}~]*$/; |
There was a problem hiding this comment.
Make sure to add a test that %20 should round trip properly, e.g. decode(encode('%20')) should work.
stringifyCookieOptimizationMeasured on Node 24.15.0.
masterwith the same benchmark rowsperf/encode-fast-pathBefore
emptysimplerfc cookie-octetsencodeundefined valuesmixed encode10 cookies100 cookiesAfter
emptysimplerfc cookie-octetsencodeundefined valuesmixed encode10 cookies100 cookiesGain Summary
emptysimplerfc cookie-octetsencodeundefined valuesmixed encode10 cookies100 cookiesTotals
Encoded Totals
Notes
This optimizes the common case where cookie values are already valid RFC 6265 cookie-octets.
The
rfc cookie-octetscase uses a value that the oldnoEncodeRegExphad to encode but the newcookieOctetRegExpcan return as-is.Values that need encoding can be slower because the new path does
cookieOctetRegExp.test(str)beforeencodeURIComponent(str).Summary: