Allow changing MAX_FRAME_SIZE. (#69)

Motivation:

SETTINGS_MAX_FRAME_SIZE is changeable: we should allow that change to
occur.

Modifications:

- Added fields for communicating MAX_FRAME_SIZE changes.
- Update the encoders/decoders as those settings change.
- Throw errors from the encoders/decoders if violated.

Result:

Users and remote peers can change MAX_FRAMES_SIZE.

Author: Lukasa

Sources/NIOHTTP2/ConnectionStateMachine/HasLocalSettings.swift

@@ -63,8 +63,7 @@ extension HasLocalSettings {
                     // respect that possibility.
                     effect.streamWindowSizeChange += Int(delta)
                 case .maxFrameSize:
-                    // TODO(cory): Implement!
-                    break
+                   effect.newMaxFrameSize = newValue
                 default:
                     // No operation required
                     return

Sources/NIOHTTP2/ConnectionStateMachine/HasRemoteSettings.swift

@@ -63,8 +63,7 @@ extension HasRemoteSettings {
                     // respect that possibility.
                     effect.streamWindowSizeChange += Int(delta)
                 case .maxFrameSize:
-                    // TODO(cory): Implement!
-                    break
+                    effect.newMaxFrameSize = newValue
                 default:
                     // No operation required
                     return

Sources/NIOHTTP2/HTTP2ChannelHandler.swift

@@ -464,10 +464,17 @@ extension NIOHTTP2Handler {
             if settingsChange.streamWindowSizeChange != 0 {
                 self.outboundBuffer.initialWindowSizeChanged(settingsChange.streamWindowSizeChange)
             }
+            if let newMaxFrameSize = settingsChange.newMaxFrameSize {
+                self.frameEncoder.maxFrameSize = newMaxFrameSize
+                self.outboundBuffer.maxFrameSize = Int(newMaxFrameSize)
+            }
         case .localSettingsChanged(let settingsChange):
             if settingsChange.streamWindowSizeChange != 0 {
                 self.inboundEventBuffer.pendingUserEvent(NIOHTTP2BulkStreamWindowChangeEvent(delta: settingsChange.streamWindowSizeChange))
             }
+            if let newMaxFrameSize = settingsChange.newMaxFrameSize {
+                self.frameDecoder.maxFrameSize = newMaxFrameSize
+            }
         }
     }
 

Sources/NIOHTTP2/HTTP2ConnectionStateChange.swift

@@ -146,6 +146,8 @@ internal enum NIOHTTP2ConnectionStateChange: Hashable {
     /// SETTINGS frame.
     internal struct RemoteSettingsChanged: Hashable {
         internal var streamWindowSizeChange: Int = 0
+
+        internal var newMaxFrameSize: UInt32?
     }
 
     /// The local peer's settings have changed in a way that is not trivial to decode.
@@ -154,6 +156,8 @@ internal enum NIOHTTP2ConnectionStateChange: Hashable {
     /// SETTINGS frame.
     internal struct LocalSettingsChanged: Hashable {
         internal var streamWindowSizeChange: Int = 0
+
+        internal var newMaxFrameSize: UInt32?
     }
 }
 

Sources/NIOHTTP2/HTTP2FrameParser.swift

@@ -228,6 +228,9 @@ struct HTTP2FrameDecoder {
     private var state: ParserState
     private var allocator: ByteBufferAllocator
 
+    // RFC 7540 § 6.5.2 puts the initial value of SETTINGS_MAX_FRAME_SIZE at 2**14 octets
+    internal var maxFrameSize: UInt32 = 1<<14
+
     /// Creates a new HTTP2 frame decoder.
     ///
     /// - parameter allocator: A `ByteBufferAllocator` used when accumulating blocks of data
@@ -319,6 +322,11 @@ struct HTTP2FrameDecoder {
                 return .needMoreData
             }
 
+            // Confirm that SETTINGS_MAX_FRAME_SIZE is respected.
+            guard header.length <= self.maxFrameSize else {
+                throw InternalError.codecError(code: .frameSizeError)
+            }
+
             if header.type != 0 {
                 // Not a DATA frame.
                 self.state = .accumulatingData(AccumulatingPayloadParserState(fromIdle: state, header: header))
@@ -924,6 +932,9 @@ struct HTTP2FrameEncoder {
     private let allocator: ByteBufferAllocator
     var headerEncoder: HPACKEncoder
 
+    // RFC 7540 § 6.5.2 puts the initial value of SETTINGS_MAX_FRAME_SIZE at 2**14 octets
+    var maxFrameSize: UInt32 = 1<<14
+
     init(allocator: ByteBufferAllocator) {
         self.allocator = allocator
         self.headerEncoder = HPACKEncoder(allocator: allocator)
@@ -1125,6 +1136,11 @@ struct HTTP2FrameEncoder {
             extraFrameData = nil
         }
 
+        // Confirm we're not about to violate SETTINGS_MAX_FRAME_SIZE.
+        guard payloadSize <= Int(self.maxFrameSize) else {
+            throw InternalError.codecError(code: .frameSizeError)
+        }
+
         // Write the frame data. This is the payload size and the flags byte.
         buf.writePayloadSize(payloadSize, at: start)
         buf.setInteger(flags.rawValue, at: flagsIndex)

Tests/NIOHTTP2Tests/HTTP2FrameParserTests+XCTest.swift

@@ -38,8 +38,10 @@ extension HTTP2FrameParserTests {
                 ("testDataFrameDecodingWithOnlyPaddingSplitOverBuffers", testDataFrameDecodingWithOnlyPaddingSplitOverBuffers),
                 ("testDataFrameEncoding", testDataFrameEncoding),
                 ("testDataFrameEncodingMultibyteLength", testDataFrameEncodingMultibyteLength),
+                ("testDataFrameEncodingViolatingMaxFrameSize", testDataFrameEncodingViolatingMaxFrameSize),
                 ("testDataFrameDecodeFailureRootStream", testDataFrameDecodeFailureRootStream),
                 ("testDataFrameDecodeFailureExcessPadding", testDataFrameDecodeFailureExcessPadding),
+                ("testDataFrameDecodingViolatingMaxFrameSize", testDataFrameDecodingViolatingMaxFrameSize),
                 ("testHeadersFrameDecodingNoPriorityNoPadding", testHeadersFrameDecodingNoPriorityNoPadding),
                 ("testHeadersFrameDecodingNoPriorityWithPadding", testHeadersFrameDecodingNoPriorityWithPadding),
                 ("testHeadersFrameDecodingWithPriorityNoPadding", testHeadersFrameDecodingWithPriorityNoPadding),

Tests/NIOHTTP2Tests/HTTP2FrameParserTests.swift

@@ -460,6 +460,22 @@ class HTTP2FrameParserTests: XCTestCase {
         XCTAssertEqual(buf, expectedBufContent)
     }
 
+
+    func testDataFrameEncodingViolatingMaxFrameSize() throws {
+        var encoder = HTTP2FrameEncoder(allocator: self.allocator)
+        XCTAssertEqual(encoder.maxFrameSize, 16384)
+
+        var content = self.allocator.buffer(capacity: 16385)
+        content.writeBytes(repeatElement(UInt8(0), count: 16385))
+
+        let frame = HTTP2Frame(streamID: 1, payload: .data(.init(data: .byteBuffer(content))))
+        var target = self.allocator.buffer(capacity: 1024)
+
+        XCTAssertThrowsError(try encoder.encode(frame: frame, to: &target)) { error in
+            XCTAssertEqual(error as? InternalError, .codecError(code: .frameSizeError))
+        }
+    }
+
     func testDataFrameDecodeFailureRootStream() {
         let frameBytes: [UInt8] = [
             0x00, 0x00, 0x01,           // 3-byte payload length (1 byte)
@@ -500,6 +516,23 @@ class HTTP2FrameParserTests: XCTestCase {
             }
         })
     }
+
+    func testDataFrameDecodingViolatingMaxFrameSize() throws {
+        let frameBytes: [UInt8] = [
+            0x00, 0x40, 0x01,           // 3-byte payload length (16385 bytes)
+            0x00,                       // 1-byte frame type (DATA)
+            0x01,                       // 1-byte flags (END_STREAM)
+            0x00, 0x00, 0x00, 0x01,     // 4-byte stream identifier
+        ]
+        var badFrameBuf = byteBuffer(withBytes: frameBytes)
+        var decoder = HTTP2FrameDecoder(allocator: self.allocator, expectClientMagic: false)
+        XCTAssertEqual(decoder.maxFrameSize, 16384)
+
+        decoder.append(bytes: &badFrameBuf)
+        XCTAssertThrowsError(try decoder.nextFrame()) { error in
+            XCTAssertEqual(error as? InternalError, .codecError(code: .frameSizeError))
+        }
+    }
 
     // MARK: - HEADERS frames
 

Tests/NIOHTTP2Tests/SimpleClientServerTests+XCTest.swift

@@ -52,6 +52,7 @@ extension SimpleClientServerTests {
                 ("testOpeningWindowsViaSettingsInitialWindowSize", testOpeningWindowsViaSettingsInitialWindowSize),
                 ("testSettingsAckNotifiesAboutChangedFlowControl", testSettingsAckNotifiesAboutChangedFlowControl),
                 ("testStreamMultiplexerAcknowledgesSettingsBasedFlowControlChanges", testStreamMultiplexerAcknowledgesSettingsBasedFlowControlChanges),
+                ("testChangingMaxFrameSize", testChangingMaxFrameSize),
            ]
    }
 }

Tests/NIOHTTP2Tests/SimpleClientServerTests.swift

@@ -1170,4 +1170,27 @@ class SimpleClientServerTests: XCTestCase {
         XCTAssertNoThrow(try self.clientChannel.finish())
         XCTAssertNoThrow(try self.serverChannel.finish())
     }
+
+    func testChangingMaxFrameSize() throws {
+        // Begin by getting the connection up.
+        try self.basicHTTP2Connection()
+
+        // We're now going to try to send a request from the client to the server.
+        let headers = HPACKHeaders([(":path", "/"), (":method", "POST"), (":scheme", "https"), (":authority", "localhost")])
+        let clientStreamID = HTTP2StreamID(1)
+        let reqFrame = HTTP2Frame(streamID: clientStreamID, payload: .headers(.init(headers: headers)))
+        try self.assertFramesRoundTrip(frames: [reqFrame], sender: self.clientChannel, receiver: self.serverChannel)
+
+        // Now the server is going to change SETTINGS_MAX_FRAME_SIZE. We set this twice to confirm we do this properly.
+        try self.assertSettingsUpdateWithAck([HTTP2Setting(parameter: .maxFrameSize, value: 1<<15), HTTP2Setting(parameter: .maxFrameSize, value: 1<<16)], sender: self.serverChannel, receiver: self.clientChannel)
+
+        // Now the client will send a very large DATA frame. It must be passed through as a single entity, and accepted by the remote peer.
+        var buffer = self.clientChannel.allocator.buffer(capacity: 65535)
+        buffer.writeBytes(repeatElement(UInt8(0xFF), count: 65535))
+        let bodyFrame = HTTP2Frame(streamID: clientStreamID, payload: .data(.init(data: .byteBuffer(buffer))))
+        try self.assertFramesRoundTrip(frames: [bodyFrame], sender: self.clientChannel, receiver: self.serverChannel)
+
+        XCTAssertNoThrow(try self.clientChannel.finish())
+        XCTAssertNoThrow(try self.serverChannel.finish())
+    }
 }