Repository navigation
Commit 6a88d8e
committed
Parse the WWW-Authenticate challenge in linear time
## Motivation and Context
`Discovery.parse_www_authenticate` walked the Bearer challenge's parameters by slicing the header at the cursor and trimming
the slice for every `key=value` pair, which copies the remainder of the header each time. The cost therefore grew with
the square of the header's length, and the header is the server's to choose: parsing one with 200,000 parameters took over 20 seconds,
on every request that drew such a `401` or `403`, before the client could act on it.
The walk now uses a `StringScanner`, which matches each pair at the current position without copying, so the cost grows with
the length of the header. The scanner locates the Bearer challenge as well, so a header whose earlier challenge holds
multibyte text yields the same parameters as before.
The header is also the server's to fill. A value whose bytes are not valid in the string's encoding, such as a header tagged UTF-8
that carries a stray `0xFF`, made the patterns raise `ArgumentError` from inside the transport's `401` handling, ahead of
any `AuthorizationError` a caller is prepared for; the default adapter hands header values over as ASCII-8BIT, where every byte is valid,
so it took a caller-supplied adapter or a header re-tagged along the way. The header is now scrubbed before it is parsed, so such bytes
become the replacement character and the other parameters still come through, while a binary header is unchanged by the scrub.
## How Has This Been Tested?
New tests in `test/mcp/client/oauth/discovery_test.rb` parse a header with 200,000 parameters within a bound the previous implementation
exceeded many times over, and a header whose earlier challenge holds multibyte text. Two more parse a UTF-8 header with an invalid byte
inside a quoted value, which raised against the previous library, and a binary header with a high byte.
## Breaking Changes
None.1 parent 6da3009 commit 6a88d8e
2 files changed
Lines changed: 62 additions & 16 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
| 4 | + | |
4 | 5 | | |
5 | 6 | | |
6 | 7 | | |
| |||
43 | 44 | | |
44 | 45 | | |
45 | 46 | | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
46 | 53 | | |
47 | 54 | | |
48 | 55 | | |
| |||
55 | 62 | | |
56 | 63 | | |
57 | 64 | | |
58 | | - | |
59 | | - | |
60 | | - | |
61 | | - | |
62 | 65 | | |
63 | | - | |
64 | | - | |
65 | | - | |
66 | | - | |
67 | | - | |
68 | | - | |
69 | | - | |
70 | | - | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
71 | 72 | | |
72 | | - | |
73 | | - | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
74 | 78 | | |
75 | | - | |
76 | | - | |
| 79 | + | |
77 | 80 | | |
78 | 81 | | |
79 | 82 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
79 | 79 | | |
80 | 80 | | |
81 | 81 | | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
82 | 125 | | |
83 | 126 | | |
84 | 127 | | |
| |||
0 commit comments