-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add option for setting max query params #5868
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
base: master
Are you sure you want to change the base?
Conversation
1841516 to
3b96aa0
Compare
|
thanks @geoand I am actually wondering why we have such limit, this is probably due to the decoder we use, but in practice I think this should be unbounded because I don't see this limit preventing any kind o DOS kind of style attack |
When develoiping this patch, I was wondering the same :) |
maybe we should simply configure the parser with the highest possible limit and spare us supporting an un-necessary configuration knob ? |
|
If you believe that is safe, I'm fine with that |
|
it is safe because the request URI we use is already limited by the keep the |
|
The Netty code mentions |
|
@geoand let me have a look |
|
@geoand let be safe and proceed as originally intended by this PR |
|
FWIW, I agree |
|
@vietj is there anything more you want me to do about this? |
|
the Eclipse Contributor Agreement validation seems to fail, do you have a valid eclipse account ? |
|
I thought I did as I have done this in the past... |
|
can you double check that ? or what account you have used ? |
|
Yes, I will check later on today |
1f802f9 to
bd2161d
Compare
bd2161d to
3a28cfc
Compare


Relates to: quarkusio/quarkus#47431