Hello Tim,
On 29/10/2021 16:57, Tim Düsterhus wrote:
Willy,
On 10/29/21 8:50 AM, Willy Tarreau wrote:
I don't see how this can ever match:
- we search for a space in the <len> first characters starting at
<ptr>
- if we find one such space, we check if these characters are exactly
equal to the string "Bearer" (modulo the case), and if so we
take the
value.
=> by definition this cannot match since there is no space in
"Bearer"
It made me doubt about strncasecmp() to the point that I had to write
some
code to verify I wasn't mistaken about how it worked.
Rémi, am I missing something or is it just that this code snippet indeed
has a bug that was not spotted by the regtests (which I'm fine with,
they're regression tests, not unit tests seeking 100% coverage) ?
You are not missing anything. This is indeed broken, because no
reg-test covers calling the `http_auth_bearer` fetch with a string
parameter.
Thus the tests always go into the 'else' part where `get_http_auth()`
handles the parsing.
The correct fix would then of course be that the parsing logic is
moved into a dedicated function (ideally based on the ist API) and not
copied into several different functions.
I retract my patch.
Remi: Shall I file an issue to track the duplicated parsing logic or
will you handle this based on this ML thread?
Willy and I talked about this together but I forgot to add the list when
I first replied to him. Some patches are waiting for a review already,
including yours.
Rémi