Allow inserting overlarge indexed headers. (#99)

Motivation:

Inserting an indexed header field that is too large is not an error, and
we shouldn't treat it as one. This was tripping over an example application
that hit Wikipedia.

Modifications:

- Stop throwing errors when we try to insert an overlarge header to the
    dynamic table.
- Add a regression test.

Result:

We can speak to Wikipedia

Sources/NIOHPACK/DynamicHeaderTable.swift

@@ -123,14 +123,7 @@ struct DynamicHeaderTable {
     ///   - name: A String representing the name of the header field.
     ///   - value: A String representing the value of the header field.
     /// - Returns: `true` if the header was added to the table, `false` if not.
-    mutating func addHeader(named name: String, value: String) throws {
-        do {
-            try self.storage.add(name: name, value: value)
-        } catch InternalError.unableToAddHeaderToTable(let excess) {
-            // ping the error up the stack, with more information
-            // TODO(cory): Remove the genericism here in future.
-            throw NIOHPACKErrors.FailedToAddIndexedHeader(bytesNeeded: self.storage.length + excess,
-                                                          name: name.utf8, value: value.utf8)
-        }
+    mutating func addHeader(named name: String, value: String) {
+        self.storage.add(name: name, value: value)
     }
 }

Sources/NIOHPACK/HPACKErrors.swift

@@ -108,11 +108,3 @@ public enum NIOHPACKErrors {
         public init() { }
     }
 }
-
-/// This enum covers errors that are thrown internally for messaging reasons. These should
-/// not leak.
-internal enum InternalError: Error {
-    case unableToAddHeaderToTable(excess: Int)
-}
-
-extension InternalError: Hashable { }

Sources/NIOHPACK/HeaderTables.swift

@@ -122,18 +122,20 @@ struct HeaderTableStorage {
         self.maxSize = newSize
     }
 
-    mutating func add(name: String, value: String) throws {
+    mutating func add(name: String, value: String) {
         let entry = HeaderTableEntry(name: name, value: value)
 
         var newLength = self.length + entry.length
         if newLength > self.maxSize {
             self.purge(toRelease: newLength - maxSize)
             newLength = self.length + entry.length
+        }
 
-            if newLength > self.maxSize {
-                // Danger, Will Robinson! We can't free up enough space!
-                throw InternalError.unableToAddHeaderToTable(excess: newLength - self.maxSize)
-            }
+
+        if newLength > self.maxSize {
+            // We can't free up enough space. This is not an error: RFC 7541 § 4.4 explicitly allows it.
+            // In this case, the append fails but the above purge is preserved.
+            return
         }
 
         self.headers.prepend(entry)

Sources/NIOHPACK/IndexedHeaderTable.swift

@@ -134,7 +134,8 @@ public struct IndexedHeaderTable {
     ///   - value: The value of the header to insert.
     /// - Returns: `true` if the header was added to the table, `false` if not.
     public mutating func add(headerNamed name: String, value: String) throws {
-        try self.dynamicTable.addHeader(named: name, value: value)
+        // This function is unnecessarily marked throws, but none of its underlying functions throw anymore.
+        self.dynamicTable.addHeader(named: name, value: value)
     }
 
     /// Appends a header to the table.

Tests/LinuxMain.swift

@@ -33,6 +33,7 @@ import XCTest
          testCase(ConnectionStateMachineTests.allTests),
          testCase(HPACKCodingTests.allTests),
          testCase(HPACKIntegrationTests.allTests),
+         testCase(HPACKRegressionTests.allTests),
          testCase(HTTP2FrameParserTests.allTests),
          testCase(HTTP2StreamMultiplexerTests.allTests),
          testCase(HTTP2ToHTTP1CodecTests.allTests),

Tests/NIOHPACKTests/HPACKRegressionTests+XCTest.swift

@@ -0,0 +1,33 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the SwiftNIO open source project
+//
+// Copyright (c) 2017-2018 Apple Inc. and the SwiftNIO project authors
+// Licensed under Apache License v2.0
+//
+// See LICENSE.txt for license information
+// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+//===----------------------------------------------------------------------===//
+//
+// HPACKRegressionTests+XCTest.swift
+//
+import XCTest
+
+///
+/// NOTE: This file was generated by generate_linux_tests.rb
+///
+/// Do NOT edit this file directly as it will be regenerated automatically when needed.
+///
+
+extension HPACKRegressionTests {
+
+   static var allTests : [(String, (HPACKRegressionTests) -> () throws -> Void)] {
+      return [
+                ("testWikipediaHeaders", testWikipediaHeaders),
+           ]
+   }
+}
+

Tests/NIOHPACKTests/HPACKRegressionTests.swift

@@ -0,0 +1,41 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the SwiftNIO open source project
+//
+// Copyright (c) 2017-2018 Apple Inc. and the SwiftNIO project authors
+// Licensed under Apache License v2.0
+//
+// See LICENSE.txt for license information
+// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+//===----------------------------------------------------------------------===//
+
+import XCTest
+import NIO
+import NIOHPACK
+
+class HPACKRegressionTests: XCTestCase {
+
+    let allocator = ByteBufferAllocator()
+
+    private func buffer<C: Collection>(wrapping bytes: C) -> ByteBuffer where C.Element == UInt8 {
+        var buffer = allocator.buffer(capacity: bytes.count)
+        buffer.writeBytes(bytes)
+        return buffer
+    }
+
+    func testWikipediaHeaders() throws {
+        // This is a simplified regression test for a bug we found when hitting Wikipedia.
+        var headerBuffer = self.buffer(wrapping: [
+            0x20,  // Header table size change to 0, which is the source of the bug.
+            0x88,  // :status 200
+            0x61, 0x96, 0xdf, 0x69, 0x7e, 0x94, 0x0b, 0x8a, 0x43, 0x5d, 0x8a, 0x08, 0x01, 0x7d, 0x40, 0x3d,
+            0x71, 0xa6, 0x6e, 0x32, 0xd2, 0x98, 0xb4, 0x6f,  // An indexable date header, which triggers the bug.
+        ])
+        var decoder = HPACKDecoder(allocator: ByteBufferAllocator())
+        let decoded = try decoder.decodeHeaders(from: &headerBuffer)
+        XCTAssertEqual(decoded, HPACKHeaders([(":status", "200"), ("date", "Tue, 16 Apr 2019 08:43:34 GMT")]))
+    }
+}