Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use URLSearchParams for cookies #181

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Use URLSearchParams for cookies #181

wants to merge 1 commit into from

Conversation

blakeembrey
Copy link
Member

@blakeembrey blakeembrey commented Oct 7, 2024

Another RFC (cc @gurgunday). This would resolve #60, but is a larger breaking change. There is a perf improvement that kicks in when you have a couple of cookies:

After:

     name                   hz     min      max    mean     p75     p99    p995    p999     rme  samples
   · simple       8,964,811.79  0.0000   2.5364  0.0001  0.0001  0.0002  0.0002  0.0006  ±1.22%  4482407   fastest
   · decode       4,232,300.93  0.0001   0.3367  0.0002  0.0003  0.0003  0.0003  0.0007  ±0.54%  2116151
   · unquote      8,297,323.52  0.0000   2.3235  0.0001  0.0001  0.0002  0.0002  0.0006  ±1.79%  4148662
   · duplicates   1,817,425.11  0.0004   0.5865  0.0006  0.0005  0.0007  0.0009  0.0039  ±1.46%   908713
   · 10 cookies     957,506.38  0.0008  12.1083  0.0010  0.0010  0.0014  0.0023  0.0082  ±4.79%   478754
   · 100 cookies     99,967.43  0.0089   1.4323  0.0100  0.0097  0.0152  0.0204  0.1920  ±0.87%    49984   slowest
   ✓ parse top-sites (15) 23289ms
     name                                  hz     min      max    mean     p75     p99    p995    p999     rme  samples
   · parse accounts.google.com   8,130,898.42  0.0000   0.6541  0.0001  0.0001  0.0002  0.0002  0.0005  ±0.63%  4065450
   · parse apple.com             7,994,050.91  0.0000   2.1091  0.0001  0.0001  0.0002  0.0003  0.0005  ±1.47%  3997026
   · parse cloudflare.com        7,744,929.04  0.0000   0.3438  0.0001  0.0001  0.0002  0.0002  0.0006  ±0.81%  3872465
   · parse docs.google.com       7,560,510.92  0.0000   0.4241  0.0001  0.0001  0.0002  0.0002  0.0007  ±1.24%  3781565
   · parse drive.google.com      7,732,243.37  0.0000   0.3429  0.0001  0.0001  0.0002  0.0002  0.0006  ±0.85%  3866122
   · parse en.wikipedia.org      2,670,356.67  0.0002   3.0270  0.0004  0.0003  0.0005  0.0005  0.0016  ±2.26%  1335179
   · parse linkedin.com          2,597,025.14  0.0002  12.9887  0.0004  0.0003  0.0005  0.0006  0.0021  ±5.22%  1316052
   · parse maps.google.com       5,192,686.55  0.0000   0.4351  0.0002  0.0002  0.0003  0.0003  0.0008  ±0.87%  2596344
   · parse microsoft.com         4,342,346.32  0.0001   0.3217  0.0002  0.0002  0.0003  0.0003  0.0005  ±0.25%  2171174
   · parse play.google.com       7,750,341.46  0.0000   0.3735  0.0001  0.0001  0.0002  0.0002  0.0006  ±0.92%  3875171
   · parse support.google.com    4,961,085.78  0.0001   0.3541  0.0002  0.0002  0.0003  0.0003  0.0007  ±1.10%  2480543
   · parse www.google.com        4,262,486.99  0.0001   0.1232  0.0002  0.0002  0.0003  0.0003  0.0008  ±0.12%  2131244
   · parse youtu.be              2,283,153.29  0.0003   0.4132  0.0004  0.0004  0.0005  0.0005  0.0010  ±1.05%  1141577
   · parse youtube.com           2,165,589.67  0.0003   8.0903  0.0005  0.0005  0.0006  0.0007  0.0049  ±3.25%  1082796   slowest
   · parse example.com          15,655,874.12  0.0000   0.2905  0.0001  0.0001  0.0001  0.0001  0.0002  ±0.88%  7827938   fastest

Before:

   · simple       9,341,825.03  0.0000   1.2480  0.0001  0.0001  0.0002  0.0002  0.0003  ±0.96%  4670913   fastest
   · decode       4,346,079.97  0.0001   0.0410  0.0002  0.0002  0.0003  0.0003  0.0004  ±0.09%  2173040
   · unquote      8,806,715.92  0.0000   0.5785  0.0001  0.0001  0.0002  0.0002  0.0003  ±1.41%  4403359
   · duplicates   1,820,531.54  0.0004  11.6393  0.0005  0.0005  0.0007  0.0007  0.0010  ±4.56%   910266
   · 10 cookies     735,642.68  0.0011   0.2965  0.0014  0.0013  0.0017  0.0020  0.0097  ±0.51%   367830
   · 100 cookies     65,319.12  0.0138   1.6775  0.0153  0.0149  0.0210  0.0279  0.1976  ±1.18%    32660   slowest
   ✓ parse top-sites (15) 23701ms
     name                                  hz     min     max    mean     p75     p99    p995    p999     rme   samples
   · parse accounts.google.com   7,631,453.22  0.0000  0.2153  0.0001  0.0001  0.0002  0.0002  0.0003  ±0.20%   3815727
   · parse apple.com             8,182,465.90  0.0000  0.3538  0.0001  0.0001  0.0002  0.0002  0.0004  ±1.14%   4091233
   · parse cloudflare.com        7,691,665.22  0.0000  0.4449  0.0001  0.0001  0.0002  0.0002  0.0006  ±1.05%   3845833
   · parse docs.google.com       7,586,838.33  0.0000  0.0495  0.0001  0.0001  0.0002  0.0002  0.0003  ±0.08%   3793420
   · parse drive.google.com      7,657,722.87  0.0000  0.3502  0.0001  0.0001  0.0002  0.0002  0.0003  ±0.17%   3828862
   · parse en.wikipedia.org      2,033,850.58  0.0004  0.2433  0.0005  0.0005  0.0006  0.0006  0.0007  ±0.18%   1016926
   · parse linkedin.com          2,261,461.69  0.0003  0.0420  0.0004  0.0005  0.0006  0.0006  0.0010  ±0.08%   1130731
   · parse maps.google.com       4,777,333.38  0.0001  0.3890  0.0002  0.0002  0.0002  0.0003  0.0005  ±0.99%   2389888
   · parse microsoft.com         3,362,181.42  0.0002  0.4966  0.0003  0.0003  0.0004  0.0004  0.0008  ±0.86%   1681091
   · parse play.google.com       7,873,235.34  0.0000  0.3467  0.0001  0.0001  0.0002  0.0002  0.0005  ±0.80%   3936618
   · parse support.google.com    5,617,905.92  0.0001  0.2164  0.0002  0.0002  0.0002  0.0003  0.0006  ±0.16%   2808953
   · parse www.google.com        3,805,747.86  0.0001  0.4552  0.0003  0.0002  0.0003  0.0004  0.0008  ±1.07%   1902874
   · parse youtu.be              1,925,563.61  0.0004  0.5719  0.0005  0.0005  0.0007  0.0007  0.0011  ±0.26%    962785   slowest
   · parse youtube.com           2,014,394.45  0.0004  0.0596  0.0005  0.0005  0.0006  0.0006  0.0023  ±0.13%   1007198
   · parse example.com          21,494,893.44  0.0000  0.3242  0.0000  0.0000  0.0001  0.0001  0.0002  ±0.18%  10747447   fastest

@gurgunday
Copy link
Contributor

gurgunday commented Oct 8, 2024

This would insta-break everyone and is too big of a change I think, but you are the maintainer 😄

I actually wanted to ask about this, why not just do this to fix #60:

-    // only assign once
-    if (!__hasOwnProperty.call(obj, key)) {
      let valStartIdx = startIndex(str, eqIdx + 1, endIdx);
      let valEndIdx = endIndex(str, endIdx, valStartIdx);

      if (
        str.charCodeAt(valStartIdx) === 0x22 /* " */ &&
        str.charCodeAt(valEndIdx - 1) === 0x22 /* " */
      ) {
        valStartIdx++;
        valEndIdx--;
      }

      const val = str.slice(valStartIdx, valEndIdx);
      obj[key] = tryDecode(val, dec);
-    }

Cookie: name=value; name=value2;

We'd have name: value2, which makes more sense to me than taking the first

@blakeembrey
Copy link
Member Author

I actually wanted to ask about this

I have no preference over keeping last vs first, but I'd be in favor of removing an extra check every iteration (with the possible downside of an extra decode). It seems unordered though, so I don't think that will solve #60. You'd need a way to getAll.

@blakeembrey
Copy link
Member Author

I think this is the relevant piece:

2.  The user agent SHOULD sort the cookie-list in the following
       order:

       *  Cookies with longer paths are listed before cookies with
          shorter paths.

       *  Among cookies that have equal-length path fields, cookies with
          earlier creation-times are listed before cookies with later
          creation-times.

       NOTE: Not all user agents sort the cookie-list in this order, but
       this order reflects common practice when this document was
       written, and, historically, there have been servers that
       (erroneously) depended on this order.

I think you'd want the longest path (most specific) cookie?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reading duplicated cookies
2 participants