HTTPCLIENT-2352: Improved flow control window handling to prevent overflow/underflow

[arturobernalg]

This PR addresses issues related to the flow control window management in the HTTP/2 implementation of Apache HttpClient. Specifically, it ensures that flow control window updates do not cause ArithmeticException due to overflow or underflow.

[garydgregory]

I don't think we should write to the console.

[ok2c]

@arturobernalg I am sorry but this is really not a good idea. We cannot just disable the data window overflow check and hope for the best. HTTP/2 endpoints can make mistakes in calculating their data windows. We need to detect those mistakes and report them as protocol violations

[arturobernalg]

@arturobernalg I am sorry but this is really not a good idea. We cannot just disable the data window overflow check and hope for the best. HTTP/2 endpoints can make mistakes in calculating their data windows. We need to detect those mistakes and report them as protocol violations

@ok2c I see your point about not bypassing the overflow check. How about keeping the protocol violation handling by throwing an H2ConnectionException on overflow? This way, we still catch misbehaving endpoints without silently ignoring the issue.

    private int updateWindow(final AtomicInteger window, final int delta) throws H2ConnectionException {
        for (;;) {
            final int current = window.get();
            final long newValue = (long) current + delta;

            if (newValue > Integer.MAX_VALUE || newValue < Integer.MIN_VALUE) {
                throw new H2ConnectionException(H2Error.FLOW_CONTROL_ERROR,
                        "Flow control window exceeded: " + newValue);
            }

            if (window.compareAndSet(current, (int) newValue)) {
                return (int) newValue;
            }
        }
    }

[ok2c]

        if (newValue > Integer.MAX_VALUE || newValue < Integer.MIN_VALUE) {
            throw new H2ConnectionException(H2Error.FLOW_CONTROL_ERROR,
                    "Flow control window exceeded: " + newValue);
        }

@arturobernalg This is hardly any different to what we have now, slightly more readable and slightly less efficient.

I would like to see protocol logs with the exception and ideally a reproducer before any changes are made.

[arturobernalg]

        if (newValue > Integer.MAX_VALUE || newValue < Integer.MIN_VALUE) {
            throw new H2ConnectionException(H2Error.FLOW_CONTROL_ERROR,
                    "Flow control window exceeded: " + newValue);
        }

@arturobernalg This is hardly any different to what we have now, slightly more readable and slightly less efficient.

I would like to see protocol logs with the exception and ideally a reproducer before any changes are made.

Fair enough @ok2c