Normalize allowed request headers and store them in a sorted set (fixes #170)

[jub0bs]

Proposed solution to issue #170

The Fetch standard provides several guarantees that we can take advantage of:

• preflight requests contain at most one Access-Control-Request-Headers header (called ACRH below);
• the names of HTTP headers, at least in the context of the CORS protocol, are case-insensitive;
• the values listed in ACRH are lowercase;
• the values listed in ACRH are unique;
• the values listed in ACRH are sorted in lexicographical order.

Therefore,

• we can ignore ACRH headers beyond the first one (if any);
• we can lowercase the allowed request headers and store the results in a sorted set (rather than in a slice).

This is the approach I followed in jub0bs/cors. 😇

Benchmarks

Here's a benchmark comparison between before (commit ad0e722) and after my fix:

Looks like a net win:

goos: darwin
goarch: amd64
pkg: github.com/rs/cors
cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
                           │     before     │                 after                 │
                           │     sec/op     │    sec/op      vs base                │
Without-8                      5.290n ± 15%    4.528n ± 31%        ~ (p=0.183 n=10)
Default-8                      92.34n ±  3%    95.86n ±  3%   +3.81% (p=0.043 n=10)
AllowedOrigin-8                126.8n ±  5%    128.6n ±  3%        ~ (p=0.541 n=10)
Preflight-8                    308.3n ±  1%    295.8n ±  1%   -4.07% (p=0.000 n=10)
PreflightHeader-8              366.5n ±  1%    333.4n ±  5%   -9.02% (p=0.000 n=10)
PreflightAdversarialACRH-8   33214.5n ±  0%    360.9n ±  4%  -98.91% (p=0.000 n=10)
Wildcard/match-8               9.723n ± 16%    9.919n ± 12%        ~ (p=0.225 n=10)
Wildcard/too_short-8          0.9875n ±  0%   0.9923n ±  0%   +0.48% (p=0.034 n=10)
geomean                        82.91n          45.86n        -44.69%

                           │     before      │                after                 │
                           │      B/op       │    B/op     vs base                  │
Without-8                       0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
Default-8                       0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
AllowedOrigin-8                 0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
Preflight-8                     0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
PreflightHeader-8               0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
PreflightAdversarialACRH-8   36904.00 ± 0%     32.00 ± 0%  -99.91% (p=0.000 n=10)
Wildcard/match-8                0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
Wildcard/too_short-8            0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
geomean                                    ²               -58.58%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                           │    before    │                after                 │
                           │  allocs/op   │ allocs/op   vs base                  │
Without-8                    0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
Default-8                    0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
AllowedOrigin-8              0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
Preflight-8                  0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
PreflightHeader-8            0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
PreflightAdversarialACRH-8   4.000 ± 0%     2.000 ± 0%  -50.00% (p=0.000 n=10)
Wildcard/match-8             0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
Wildcard/too_short-8         0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
geomean                                 ²                -8.30%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

[jub0bs]

As a bonus, this PR obsoletes #133.

[jub0bs]

@rs Have you had a chance to study my changes? I think that maintainers of projects that depend on rs/cors would likely want #170 to be fixed sooner rather than later.

[rs]

It's a larger change than I expected. I have it in my TODO.

[jub0bs]

I've reduced the size of the malicious ACRH header in the benchmark. No need for it to be long to show that my PR fixes the issue.

[rs]

I can't find where in the CORS spec, it is required that the values listed in ACRH are sorted in lexicographical order. Can you please point me to where it is defined?

[rs]

Ok I see it in the fetch spec as pointed out in the comment and validated current browser are following this recommendation.

[rs]

Thanks fir PR. Great implementation.