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

Request Header Fields Too Large when using combined with auth0/next-auth #67

Closed
joakimeliasson opened this issue Oct 3, 2022 · 14 comments

Comments

@joakimeliasson
Copy link

We are getting 431 (Request Header Fields Too Large) when using next-plausible and auth0/nextjs-auth. The auth library sets a couple of http only cookies for an encrypted session which are quite large (size 4000 and 3300 respectively).

Is there a setting or a workaround for this problem that you are aware of?

Thanks!

@grempe
Copy link

grempe commented Oct 14, 2022

I came here for exactly this issue as well.

Requests to https://www.mydomain.com/proxy/api/event are throwing a 431 error.

Also Next.js app using auth0/nextjs-auth

@vinibrsl
Copy link

Currently, Plausible's Phoenix server has a default limit on the size of the request headers and cookies. I'm guessing that when proxying the /api/event request, all browser cookies are also being sent, and this request does not need any cookies.

We're having internal discussions on increasing the request header permitted size, but in the meantime, could you please check if your request includes any cookies due to the proxy? If it includes, is dropping those cookies an option for your setup @joakimeliasson?

@grempe
Copy link

grempe commented Oct 18, 2022

If @4lejandrito could engage here it would be helpful. I have been in direct contact with the leadership at Plausible and @vinibrsl is one of their engineers who is working to help resolve this issue.

They do have a limit on request size which is being reached due to rewriting Next.js requests which send all first party cookies to their servers as well. I can understand why they would not want to have this happen in the long term (and we don't want our clients cookies being sent to them either).

I suspect that this is inherent in this solution due to the fact the Next.js rewrites are being used to 'proxy' the requests.

https://nextjs.org/docs/api-reference/next.config.js/rewrites

As I understand it, it is not possible to modify the request object on rewrite. It just gets passed through to a new destination.

This might only be able to be solved if instead of using the rewrite mechanism, an actual Next.js API endpoint is used instead. It would receive the local API request, strip it of all but the payload intended for Plausible (most importantly the cookies), and then create a new fetch request destined for Plausible's API. This is actual proxy functionality, not merely rewrite.

Thoughts?

vinibrsl added a commit to plausible/analytics that referenced this issue Oct 18, 2022
This commit makes the permitted header length more permissive, 8,192
bytes, doubling the Phoenix default.

Related to 4lejandrito/next-plausible#67
vinibrsl added a commit to plausible/analytics that referenced this issue Oct 19, 2022
This commit makes the permitted header length more permissive, 8,192
bytes, doubling the Phoenix default.

Related to 4lejandrito/next-plausible#67
@grempe
Copy link

grempe commented Oct 19, 2022

I can confirm that @vinibrsl has made a change to the default header size that was being accepted server side at Plausible.

This resolved the 431 request errors for me. Thank's to their team for being so responsive.

However, this problem should still be addressed in this library as well with a proper request proxy that strips cookies and other extraneous info from the header. Sending first party cookies from clients to Plausible is a security issue on top of an efficiency issue.

@4lejandrito
Copy link
Owner

4lejandrito commented Oct 20, 2022

Hi guys! Sorry for the late response. I'm very busy lately (had a kid). I'm glad to see @vinibrsl has helped on this and, as @grempe suggested, I'll investigate to see if we can strip the cookies directly from next-plausible.

@4lejandrito
Copy link
Owner

@vinibrsl have you considered using fetch instead of XMLHttpRequest in Plausible's js script?. This way you can set {credentials: 'omit'} to make sure no cookies are sent.

@vinibrsl
Copy link

@vinibrsl have you considered using fetch instead of XMLHttpRequest in Plausible's js script?. This way you can set {credentials: 'omit'} to make sure no cookies are sent.

Thanks for looking into this, @4lejandrito!

We try to make the script lightweight and compatible with most browsers, including old ones e.g. IE 11. That is the main reason we use XMLHttpRequest and not the fetch API.

@4lejandrito
Copy link
Owner

And isn't there a way to disable cookies in XMLHttpRequest? The closest I found is this, but it doesn't look standard. I think this should be somehow solved at the plausible script level, because even if I fix it in next-plausible you guys might face the same issue in other stacks.

@4lejandrito
Copy link
Owner

4lejandrito commented Oct 23, 2022

@vinibrsl I see you have https://plausible.io/docs/script-extensions#scriptcompatjs to support old versions of IE. Maybe you can ship fetch by default and use XMLHttpRequest in compat mode?

@vinibrsl
Copy link

We had an internal discussion with the team, and we decided not to replace XMLHttpRequest with fetch for now, as we try to be as careful as possible when introducing a breaking change. Most Plausible Cloud sites use the default script which somewhat supports IE. If we change it, they'd have to move to the compat version but before that tracking would break for some time. In any case, thanks for the suggestion, I didn't know about the credentials omit option and I wish we had something like that in XMLHttpRequest that worked cross-browser.

I believe the caller is responsible for stripping out potentially sensitive information (like cookies) from the Plausible event request. We thought of removing unneeded headers from our load balancer, but that could possibly break sites, e.g. removing a cookie the site needs but Plausible does not.

To address the HTTP 431 from Plausible's side we did:

It seems like Next.js rewrites don't let you strip cookies. In that case, I suggest updating this repository README to clarify that ☺️

vinibrsl added a commit to plausible/analytics that referenced this issue Oct 26, 2022
This commit makes the permitted header length more permissive, 8,192
bytes, doubling the Phoenix default.

Related to 4lejandrito/next-plausible#67
@4lejandrito
Copy link
Owner

Hi @vinibrsl, thanks for the explanation. I'll add a disclaimer to the README then.

@4lejandrito
Copy link
Owner

See e523e55.

@4lejandrito
Copy link
Owner

Found a similar discussion in the next repo and left a comment: vercel/next.js#36987 (reply in thread).

@4lejandrito
Copy link
Owner

Just committed ff89de3 documenting a workaround using middleware, and a test that ensures it works.

I'd love to fix this transparently for every user but nextjs middleware is designed to be static and not composable. In the future I'll explore a way to do this through a custom API route, but still it would have to be manually set by users.

Since the issue is partially resolved by plausible and I've documented a workaround, I'll close this for now.

Thanks!

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

4 participants