Forbid empty :path headers. (#94)

Motivation:

RFC 7540 does not allow empty :path pseudo-headers. We should police this.

Modifications:

- Added a check that :path isn't empty.

Result:

Better RFC 7540 validation.

Author: Lukasa

Sources/NIOHTTP2/HPACKHeaders+Validation.swift

@@ -142,13 +142,23 @@ extension RequestBlockValidator: HeaderBlockValidator {
         // This is a bit awkward.
         //
         // For now we don't support extended-CONNECT, but when we do we'll need to update the logic here.
-        guard let pseudoHeaderType = pseudoHeaderType, pseudoHeaderType == .method else {
+        guard let pseudoHeaderType = pseudoHeaderType else {
             // Nothing to do here.
             return
         }
 
-        // This is a method pseudo-header. Check if the value is CONNECT.
-        self.isConnectRequest = value == "CONNECT"
+        switch pseudoHeaderType {
+        case .method:
+            // This is a method pseudo-header. Check if the value is CONNECT.
+            self.isConnectRequest = value == "CONNECT"
+        case .path:
+            // This is a path pseudo-header. It must not be empty.
+            if value.utf8.count == 0 {
+                throw NIOHTTP2Errors.EmptyPathHeader()
+            }
+        default:
+            break
+        }
     }
 
     var allowedPseudoHeaderFields: PseudoHeaders {

Sources/NIOHTTP2/HTTP2Error.swift

@@ -210,6 +210,11 @@ public enum NIOHTTP2Errors {
         public init() { }
     }
 
+    /// A HTTP/2 header block was received with an empty :path header.
+    public struct EmptyPathHeader: NIOHTTP2Error {
+        public init() { }
+    }
+
     /// A :status header was received with an invalid value.
     public struct InvalidStatusValue: NIOHTTP2Error {
         public var value: String

Tests/NIOHTTP2Tests/ConnectionStateMachineTests+XCTest.swift

@@ -91,6 +91,8 @@ extension ConnectionStateMachineTests {
                 ("testAllowRequestHeadersWithMissingMethodFieldWhenValidationDisabled", testAllowRequestHeadersWithMissingMethodFieldWhenValidationDisabled),
                 ("testRejectRequestHeadersWithMissingPathField", testRejectRequestHeadersWithMissingPathField),
                 ("testAllowRequestHeadersWithMissingPathFieldWhenValidationDisabled", testAllowRequestHeadersWithMissingPathFieldWhenValidationDisabled),
+                ("testRejectRequestHeadersWithEmptyPathField", testRejectRequestHeadersWithEmptyPathField),
+                ("testAllowRequestHeadersWithEmptyPathFieldWhenValidationDisabled", testAllowRequestHeadersWithEmptyPathFieldWhenValidationDisabled),
                 ("testRejectRequestHeadersWithMissingSchemeField", testRejectRequestHeadersWithMissingSchemeField),
                 ("testAllowRequestHeadersWithMissingSchemeFieldWhenValidationDisabled", testAllowRequestHeadersWithMissingSchemeFieldWhenValidationDisabled),
                 ("testRejectRequestHeadersWithDuplicateMethodField", testRejectRequestHeadersWithDuplicateMethodField),

Tests/NIOHTTP2Tests/ConnectionStateMachineTests.swift

@@ -1930,6 +1930,34 @@ class ConnectionStateMachineTests: XCTestCase {
         assertSucceeds(self.server.receiveHeaders(streamID: streamOne, headers: headers, isEndStreamSet: true))
     }
 
+    func testRejectRequestHeadersWithEmptyPathField() {
+        let streamOne = HTTP2StreamID(1)
+
+        self.exchangePreamble()
+
+        let headers = HPACKHeaders([(":authority", "localhost"), (":scheme", "https"), (":method", "GET"), (":path", ""), ("content-length", "0")])
+
+        // Request headers must contain non-empty :path.
+        assertStreamError(type: .protocolError, self.client.sendHeaders(streamID: streamOne, headers: headers, isEndStreamSet: true))
+        assertStreamError(type: .protocolError, self.server.receiveHeaders(streamID: streamOne, headers: headers, isEndStreamSet: true))
+    }
+
+    func testAllowRequestHeadersWithEmptyPathFieldWhenValidationDisabled() {
+        let streamOne = HTTP2StreamID(1)
+
+        // Override the setup with validation disabled.
+        self.server = .init(role: .server, headerBlockValidation: .disabled)
+        self.client = .init(role: .client, headerBlockValidation: .disabled)
+
+        self.exchangePreamble()
+
+        let headers = HPACKHeaders([(":authority", "localhost"), (":scheme", "https"), (":method", "GET"), (":path", ""), ("content-length", "0")])
+
+        // Request headers must contain non-empty :path.
+        assertSucceeds(self.client.sendHeaders(streamID: streamOne, headers: headers, isEndStreamSet: true))
+        assertSucceeds(self.server.receiveHeaders(streamID: streamOne, headers: headers, isEndStreamSet: true))
+    }
+
     func testRejectRequestHeadersWithMissingSchemeField() {
         let streamOne = HTTP2StreamID(1)