Add additional configuration for max post, max overall request body, max header sizes #43
Conversation
…cap for processing form data similar to the multipart configuration.
| if (contentType != null && contentType.equalsIgnoreCase(ContentTypes.Form)) { | ||
| byte[] body = getBodyBytes(); | ||
| HTTPTools.parseEncodedData(body, 0, body.length, formData); | ||
| HTTPTools.parseEncodedData(body, 0, body.length, getCharacterEncoding(), formData); |
There was a problem hiding this comment.
Note this was missing previously and always assumed UTF-8. This is a good assumption, and I think is even the de-facto standard.
But I was able to find plenty of reports where a different encoding was provided and the server didn't handle it. So this seems like a good thing to do. Multipart form data was correctly handling the encoding and falls back to UTF-8. I made the same change here.
See HTTPRequestTest.java and HTTPToolsTest - I added some tests for various encodings.
| } | ||
|
|
||
| // Keep existing default if one was not provided. | ||
| if (!maxRequestBodySize.containsKey("*")) { |
There was a problem hiding this comment.
Could optionally just fail and force the user to specify this value.
| * @param maxRequestBodySize the maximum request size configuration | ||
| * @return the maximum request size, or -1 if no limit should be enforced. | ||
| */ | ||
| public static int getMaxRequestBodySize(String contentType, Map<String, Integer> maxRequestBodySize) { |
There was a problem hiding this comment.
Note that I didn't use a while loop as we discussed.
The RFC says you that you must have one, and only one subtype, so the only way you could have more than one / in the Content-Type value would be if it was invalid content-type, or the multipart/ header contains a / in a boundary value. But the value we pass into this method contains only the left part of the value up until the ; so it will not contain encoding or boundary attributes.
With that in mind, this seemed easier to reason through, but open to other opinions.
| @Test | ||
| public class HTTPToolsTest { | ||
| @Test | ||
| public void getMaxRequestBodySize() { |
There was a problem hiding this comment.
Any other edge cases you can think of that I want to test for?
The values I am testing reflect what I expect at runtime which means I do not expect attributes or anything after the ; for the value of the Content-Type header.
I added some tests for good measure for values I don't expect at runtime just to make sure I don't explode.
|
Doesn't look like this repo needs approvals, but this PR looks good. Feel free to merge! |
Allow the default max size for form config to be removed so that it is possible to use a single max body size.
Summary
Add additional configuration for max post, max overall request body, max header sizes
Related