-
Notifications
You must be signed in to change notification settings - Fork 203
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
Only cap the input if content length header is given #1302
Conversation
@yegor256 anything else needed here? |
@laeubi would be great if you can reproduce the problem with a unit test too. Possible? |
I now added a testcase that fails without my patch but succeeds when the patch is applied. |
@laeubi now looks perfect, but I'm afraid there is a formatting mistake in the unit test (see what Qulice is saying). |
Is there a way to apply the desired formatting automatically? |
@laeubi we don't have that, here is why: https://www.yegor256.com/2018/01/16/educational-aspect-of-static-analysis.html |
While theoretically this might be good I contribute to literally hundreds of projects each using slightly different "better styles" so learning all of them is at least not really affordable for contributors especially now I have to count whitespace, rerun build again and even decode non trivial instructions like
that's really a mess and don't bring any value (neither to me nor to the project) as it is simply stupid work that can be automated. |
@laeubi I agree. However, we don't have a tool for that. We tried to create it in the past, but failed :( Sorry. |
Currently RqLengthAware uses the InputStream#available to cap the input if no Content-Length header is given, but the return value does not say anything about the real content length, only how many bytes can be read without blocking (what might be 0). This now only returns a CapInputStream when a Content-Length is given in the request and adds a testcase for the given case.
I have now addressed the style errors, I know at least Apache uses https://github.com/diffplug/spotless/blob/main/plugin-maven/README.md |
@laeubi it seems that something is wrong with the unit test introduced, since all workflow runs are hanging. Can you please check? |
At laest in the UI they all run fine and fast... Also in github I see they pass:
what seems hanging is the proxy test
it also hangs locally for me I suspect this is due to this bug: before my change there was always a CapInputStream returned that terminated (but maybe with wrong bytes) always, now it returns the raw input stream that possibly never returns... |
@laeubi thanks! |
Currently RqLengthAware uses the InputStream#available to cap the input if no Content-Length header is given, but the return value does not say anything about the real content length, only how many bytes can be read without blocking (what might be 0).
This now only returns a CapInputStream when a Content-Length is given in the request.
Fixes #1301