Support max http request header size and max http header size configuration separation

[CrazyHZM]

Mail link：https://bz.apache.org/bugzilla/show_bug.cgi?id=65866

[CrazyHZM]

Please review the code first, if there is no problem, I will support this on other branches as well.

[gmshake]

After the change, maxHttpHeaderSize becomes default maximum size of the HTTP message header for both request and response.

If we no longer have consumer of getMaxHttpHeaderSize(), then I'd suggest to remove maxHttpHeaderSize, and change setMaxHttpHeaderSize() like this

@Deprecated
public void setMaxHttpHeaderSize(int valueI) {
    maxHttpRequestHeaderSize = valueI;
    maxHttpResponseHeaderSize = valueI;
}

[CrazyHZM]

I'm worried about the compatibility risk of removing getMaxHttpHeaderSize(), because this is a public API after all, I wonder if this change can be done in 10.1.x. of which I agree with the annotation Deprecated.

[gmshake]

Keep getMaxHttpHeaderSize() only preserves API level compatibility, the semantic of getMaxHttpHeaderSize() has change once we introduce dedicated header settings for request and response.

To illustrate the semantic change:

setMaxHttpHeaderSize(4096);
setMaxHttpRequestHeaderSize(8192);
setMaxHttpResponseHeaderSize(16384);

Then getMaxHttpHeaderSize() make no sense except the only meaning "default maxHttpHeaderSize"

Maybe it is more reasonable to deprecate getMaxHttpHeaderSize() and return a size that is not too bad:

@Deprecated
public int getMaxHttpHeaderSize() {
    return Math.min(maxHttpRequestHeaderSize, maxHttpResponseHeaderSize)
}

I still insist that semantic level compatibility is more important than API level compatibility. For setMaxHttpHeaderSize() it originally means set maximum size of the HTTP message header for both request and response.

[rmannibucau]

Setter should stay IMHO and set both values to the exact same one if not already set.
Getter should get the max value since the consumer wants the max size and it is exactly the semantic of this method (= what is the max for request and response, only change is that before it was a single value, now it is really a max but a min is wrong IMHO).

So IMHO it makes perfect sense as method for frameworks and integrators to still have this, in particular when it is linked to pooling where max is more intended than min or any value under the max.

I still insist that semantic level compatibility is more important than API level compatibility

Well there are two points:

1. API compat: IMHO it is not an option to break before next 10.2 or something like that
2. semantic level is preserved and it makes sense to have this shortcut IMHO so we can solve both at once

[rmaucher]

That seems pretty straightforward. I'm slightly skeptical about the real practical use since trying to optimize the sizes too much is risky, but no problem, this is the user's responsibility.

[CrazyHZM]

That seems pretty straightforward. I'm slightly skeptical about the real practical use since trying to optimize the sizes too much is risky, but no problem, this is the user's responsibility.

@rmaucher
I saw some discussion about this, you can refer to it:
spring-projects/spring-boot#29382

[rmannibucau]

+1 I see two advantages:

1. enables cases where "front" tomcat has big incoming headers but "normal" forwarded headers
2. enables better web server abstractions which often have this configuration

Open question: shouldn't setMaxHttpHeaderSize set both request and response ones and maxHttpHeaderSize field be dropped for backward compat (not sure deprecating makes sense since in 80% of cases it is the same value)?

[rmaucher]

Overall, I prefer the PR as it is right now, it's simpler. If the new setters are used, then the behavior could be slightly different, but since this is new API this should be fine.

[gmshake]

@rmaucher

Overall, I prefer the PR as it is right now, it's simpler. If the new setters are used, then the behavior could be slightly different, but since this is new API this should be fine.

It is not complicated. I forked and edit online to demonstrate the part of change: gmshake@75289cf?diff=split

[rmaucher]

The proposed patch is still simpler and should work well, so I will merge that one.

[CrazyHZM]

@rmaucher
I can patch several other branches, right?

[rmaucher]

Merged with changed. I didn't understand the changes to TestHttp2Limits, HTTP/2 should use the new request header size and that should be it. Also updated docs and changelog.

[CrazyHZM]

@rmaucher
Do i need to resubmit a clean pr?