Skip to content

Commit

Permalink
Make HPACKHeaders hashable. (#344)
Browse files Browse the repository at this point in the history
Motivation:

HPACKHeaders is equatable, and it rarely makes sense to have a type that
is equatable but not hashable. The definition of equatability is pretty
limited here (see #342) but we can define a definition of hashability
consistent with it.

Modifications:

- Make HPACKHeaders hashable.

Result:

People can store HPACKHeaders in Sets

Co-authored-by: David Nadoba <dnadoba@gmail.com>
  • Loading branch information
Lukasa and dnadoba authored May 4, 2022
1 parent eb1d696 commit c28d13d
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 0 deletions.
19 changes: 19 additions & 0 deletions Sources/NIOHPACK/HPACKHeader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions Tests/LinuxMain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
34 changes: 34 additions & 0 deletions Tests/NIOHPACKTests/HPACKHeadersTests+XCTest.swift
Original file line number Diff line number Diff line change
@@ -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),
]
}
}

74 changes: 74 additions & 0 deletions Tests/NIOHPACKTests/HPACKHeadersTests.swift
Original file line number Diff line number Diff line change
@@ -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])
)
}
}

0 comments on commit c28d13d

Please sign in to comment.