Repository: apple/swift-nio-http2 (swift)

Issue:
Max frame size can be (1<<24)-1
In `validateSettings()` (ConnectionStateMachine.swift:1311)
```
            case .maxFrameSize:
                guard setting._value >= (1 << 14) && setting._value < ((1 << 24) - 1) else {
```

However, `(1 << 24) - 1` is a valid frame size. The comparison should be `setting._value < (1 << 24)`

Pull request description:
Motivation:

When we're validating that settings are in their bounds, we should probably endeavour to
express those bounds correctly.

Modifications:

- Fixed the bounds checks on SETTINGS_INITIAL_WINDOW_SIZE and SETTINGS_MAX_FRAME_SIZE.
- Added regression tests.

Result:

Better correctness.
Resolves #96

Patch:
diff --git a/Sources/NIOHTTP2/ConnectionStateMachine/ConnectionStateMachine.swift b/Sources/NIOHTTP2/ConnectionStateMachine/ConnectionStateMachine.swift
index 395b75d5..06612027 100644
--- a/Sources/NIOHTTP2/ConnectionStateMachine/ConnectionStateMachine.swift
+++ b/Sources/NIOHTTP2/ConnectionStateMachine/ConnectionStateMachine.swift
@@ -1304,11 +1304,11 @@ extension HTTP2ConnectionStateMachine {
                     return .connectionError(underlyingError: NIOHTTP2Errors.InvalidSetting(setting: setting), type: .protocolError)
                 }
             case .initialWindowSize:
-                guard setting._value < HTTP2FlowControlWindow.maxSize else {
+                guard setting._value <= HTTP2FlowControlWindow.maxSize else {
                     return .connectionError(underlyingError: NIOHTTP2Errors.InvalidSetting(setting: setting), type: .flowControlError)
                 }
             case .maxFrameSize:
-                guard setting._value >= (1 << 14) && setting._value < ((1 << 24) - 1) else {
+                guard setting._value >= (1 << 14) && setting._value <= ((1 << 24) - 1) else {
                     return .connectionError(underlyingError: NIOHTTP2Errors.InvalidSetting(setting: setting), type: .protocolError)
                 }
             default:


Test patch:
diff --git a/Tests/NIOHTTP2Tests/ConnectionStateMachineTests+XCTest.swift b/Tests/NIOHTTP2Tests/ConnectionStateMachineTests+XCTest.swift
index 154b88e0..d5776070 100644
--- a/Tests/NIOHTTP2Tests/ConnectionStateMachineTests+XCTest.swift
+++ b/Tests/NIOHTTP2Tests/ConnectionStateMachineTests+XCTest.swift
@@ -122,6 +122,8 @@ extension ConnectionStateMachineTests {
                 ("testRejectHeadersWithTEHeaderNotTrailers", testRejectHeadersWithTEHeaderNotTrailers),
                 ("testAllowHeadersWithTEHeaderNotTrailersWhenValidationDisabled", testAllowHeadersWithTEHeaderNotTrailersWhenValidationDisabled),
                 ("testAllowHeadersWithTEHeaderSetToTrailers", testAllowHeadersWithTEHeaderSetToTrailers),
+                ("testSettingActualMaxFrameSize", testSettingActualMaxFrameSize),
+                ("testSettingActualInitialWindowSize", testSettingActualInitialWindowSize),
            ]
    }
 }
diff --git a/Tests/NIOHTTP2Tests/ConnectionStateMachineTests.swift b/Tests/NIOHTTP2Tests/ConnectionStateMachineTests.swift
index d81e60d4..0d464a4d 100644
--- a/Tests/NIOHTTP2Tests/ConnectionStateMachineTests.swift
+++ b/Tests/NIOHTTP2Tests/ConnectionStateMachineTests.swift
@@ -2498,6 +2498,26 @@ class ConnectionStateMachineTests: XCTestCase {
         assertSucceeds(self.client.sendHeaders(streamID: streamOne, headers: ConnectionStateMachineTests.requestHeaders.withExtraHeaders(invalidExtraHeaders), isEndStreamSet: true))
         assertSucceeds(self.server.receiveHeaders(streamID: streamOne, headers: ConnectionStateMachineTests.requestHeaders.withExtraHeaders(invalidExtraHeaders), isEndStreamSet: true))
     }
+
+    func testSettingActualMaxFrameSize() {
+        self.exchangePreamble()
+
+        // It must be possible to set SETTINGS_MAX_FRAME_SIZE to (2**24)-1.
+        let trickySettings: HTTP2Settings = [HTTP2Setting(parameter: .maxFrameSize, value: (1<<24) - 1)]
+        assertSucceeds(self.client.sendSettings(trickySettings))
+        assertSucceeds(self.server.receiveSettings(.settings(trickySettings), frameEncoder: &self.serverEncoder, frameDecoder: &self.serverDecoder))
+        assertSucceeds(self.client.receiveSettings(.ack, frameEncoder: &self.clientEncoder, frameDecoder: &self.clientDecoder))
+    }
+
+    func testSettingActualInitialWindowSize() {
+        self.exchangePreamble()
+
+        // It must be possible to set SETTINGS_INITIAL_WINDOW_SIZE to (2**31)-1.
+        let trickySettings: HTTP2Settings = [HTTP2Setting(parameter: .initialWindowSize, value: (1<<31) - 1)]
+        assertSucceeds(self.client.sendSettings(trickySettings))
+        assertSucceeds(self.server.receiveSettings(.settings(trickySettings), frameEncoder: &self.serverEncoder, frameDecoder: &self.serverDecoder))
+        assertSucceeds(self.client.receiveSettings(.ack, frameEncoder: &self.clientEncoder, frameDecoder: &self.clientDecoder))
+    }
 }