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

Statically allocating space for 16 headers is too few for redirects from some OAuth providers #21

Closed
mrodz opened this issue May 29, 2024 · 3 comments

Comments

@mrodz
Copy link

mrodz commented May 29, 2024

let mut headers = [httparse::EMPTY_HEADER; 16];

Google's OAuth API for web apps will send more than 16 headers, which causes this line:

request.parse(&buffer).ok()?;

To throw:

thread '<unnamed>' panicked at desktop\src\auth.rs:118:28:
called `Result::unwrap()` on an `Err` value: TooManyHeaders
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

For my use case.

Is this behavior present and noticeable for other users of this plugin?

@mrodz
Copy link
Author

mrodz commented May 29, 2024

A quick note:
I did not see any stack trace when originally using this plugin directly and was only met with an OAuth redirect to a: 127.0.0.1:PORT did not send any data. net::ERR_EMPTY_RESPONSE chromium page. I only found out this was the source of the bug by vendoring the dependency and manually doing some testing.

@FabianLars
Copy link
Owner

Is this behavior present and noticeable for other users of this plugin?

For what it's worth, you're the first one to tell me about this. What's weird is that google is exactly the reason i created this plugin so it's probably the most tested oauth provider - i guess something changed.

Do you have a rough estimate on the amount of headers?

@mrodz
Copy link
Author

mrodz commented May 29, 2024

On a fresh Chrome profile, the redirect appears to send exactly 16 headers. However, some users may use browser extensions or have an application-level firewall that adds extra headers to HTTP requests, which is how I ended up with some in the 17-19 range and a TCP server crash without warning or message. I added space for 32 headers in my own app just to account for the case where there are extra headers.

Whether or not this is important enough to warrant additional space allocated in the handler function is up to you. I just wanted to bring this to your attention :)

Here are all of the incoming headers for one request:

Host=127.0.0.1:59778
Connection=keep-alive
Cache-Control=max-age=0
DNT=1
Upgrade-Insecure-Requests=1
User-Agent=******
Accept=text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7
Sec-Fetch-Site=cross-site
Sec-Fetch-Mode=navigate
Sec-Fetch-User=?1
Sec-Fetch-Dest=document
sec-ch-ua="Google Chrome";v="125", "Chromium";v="125", "Not.A/Brand";v="24"
sec-ch-ua-mobile=?0
sec-ch-ua-platform="Windows"
Referer=https://accounts.google.com/
Accept-Encoding=gzip, deflate, br, zstd
Accept-Language=en-US,en;q=0.9,es-US;q=0.8,es;q=0.7
Cookie=******
[desktop\src\auth.rs:118:5] header_c = 18

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

No branches or pull requests

2 participants