diff --git a/Sources/NIOHPACK/HPACKHeader.swift b/Sources/NIOHPACK/HPACKHeader.swift index 9b17a269..3406c98d 100644 --- a/Sources/NIOHPACK/HPACKHeader.swift +++ b/Sources/NIOHPACK/HPACKHeader.swift @@ -394,6 +394,9 @@ extension HPACKHeaders: CustomStringConvertible { } } +// NOTE: This is a bad definition of equatable and hashable. In particular, both order and +// indexability are ignored. We should change it, but we should be careful when we do so. +// More discussion at https://github.com/apple/swift-nio-http2/issues/342. extension HPACKHeaders: Equatable { @inlinable public static func ==(lhs: HPACKHeaders, rhs: HPACKHeaders) -> Bool { @@ -416,6 +419,22 @@ extension HPACKHeaders: Equatable { } } +extension HPACKHeaders: Hashable { + @inlinable + public func hash(into hasher: inout Hasher) { + // Discriminator, to indicate that this is a collection. This improves the performance + // of Sets and Dictionaries that include collections of HPACKHeaders by reducing hash collisions. + hasher.combine(self.count) + + // This emulates the logic used in equatability, but we sort it to ensure that + // we hash equivalently. + let names = Set(self.headers.lazy.map { $0.name }).sorted() + for name in names { + hasher.combine(self[name].sorted()) + } + } +} + /// Defines the types of indexing and rewriting operations a decoder may take with /// regard to this header. public enum HPACKIndexing: CustomStringConvertible { diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index e989850c..13060b0e 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -43,6 +43,7 @@ class LinuxMainRunnerImpl: LinuxMainRunner { testCase(ConnectionStateMachineTests.allTests), testCase(ControlFrameBufferTests.allTests), testCase(HPACKCodingTests.allTests), + testCase(HPACKHeadersTests.allTests), testCase(HPACKIntegrationTests.allTests), testCase(HPACKRegressionTests.allTests), testCase(HTTP2ErrorTests.allTests), diff --git a/Tests/NIOHPACKTests/HPACKHeadersTests+XCTest.swift b/Tests/NIOHPACKTests/HPACKHeadersTests+XCTest.swift new file mode 100644 index 00000000..0f45f4ad --- /dev/null +++ b/Tests/NIOHPACKTests/HPACKHeadersTests+XCTest.swift @@ -0,0 +1,34 @@ +//===----------------------------------------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// +// +// HPACKHeadersTests+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 HPACKHeadersTests { + + @available(*, deprecated, message: "not actually deprecated. Just deprecated to allow deprecated tests (which test deprecated functionality) without warnings") + static var allTests : [(String, (HPACKHeadersTests) -> () throws -> Void)] { + return [ + ("testHPACKHeadersAreHashable", testHPACKHeadersAreHashable), + ] + } +} + diff --git a/Tests/NIOHPACKTests/HPACKHeadersTests.swift b/Tests/NIOHPACKTests/HPACKHeadersTests.swift new file mode 100644 index 00000000..bee09893 --- /dev/null +++ b/Tests/NIOHPACKTests/HPACKHeadersTests.swift @@ -0,0 +1,74 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the SwiftNIO open source project +// +// Copyright (c) 2022 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 NIOCore +import NIOHPACK + +final class HPACKHeadersTests: XCTestCase { + func testHPACKHeadersAreHashable() throws { + let firstHeaders = HPACKHeaders([ + (":foo", "bar"), + (":bar", "baz"), + ("boz", "box"), + ("fox", "fro"), + ]) + // The same as firstHeaders but in a different order. + let secondHeaders = HPACKHeaders([ + (":bar", "baz"), + (":foo", "bar"), + ("fox", "fro"), + ("boz", "box"), + ]) + // Differs by one name from firstHeaders + let thirdHeaders = HPACKHeaders([ + (":foo", "bar"), + (":bar", "baz"), + ("boz", "box"), + ("fax", "fro"), + ]) + // Differs by one value from firstHeaders + let fourthHeaders = HPACKHeaders([ + (":foo", "bar"), + (":bar", "baz"), + ("boz", "box"), + ("fox", "fra"), + ]) + + // First, confirm all things hash to themselves. This proves basic hashing correctness. + XCTAssertEqual( + Set([firstHeaders, firstHeaders]), + Set([firstHeaders]) + ) + XCTAssertEqual( + Set([secondHeaders, secondHeaders]), + Set([secondHeaders]) + ) + XCTAssertEqual( + Set([thirdHeaders, thirdHeaders]), + Set([thirdHeaders]) + ) + XCTAssertEqual( + Set([fourthHeaders, fourthHeaders]), + Set([fourthHeaders]) + ) + + // Next, prove we can discriminate between different things. Here, importantly, secondHeaders is removed, as + // it hashes equal to firstHeaders. + XCTAssertEqual( + Set([firstHeaders, secondHeaders, thirdHeaders, fourthHeaders]), + Set([firstHeaders, thirdHeaders, fourthHeaders]) + ) + } +}