[RFC 9651] Add support for Display String type to RawStructuredFieldValues#43
Conversation
…alues (apple#41) Motivation: [RFC 9651](https://www.ietf.org/rfc/rfc9651.html) added the Display String Structured Type. Modifications: - Implement the parser and serializer for Display String in the RawStructuredFieldValues module. Result: The RawStructuredFieldValues module will support the Display String type.
|
|
||
| let startIndex = self.underlyingData.startIndex | ||
| let secondIndex = self.underlyingData.index(after: startIndex) | ||
| let octetHex = self.underlyingData[...secondIndex] |
There was a problem hiding this comment.
We can skip the index math and do let octetHex = self.underlyingData.prefix(2)
| self.underlyingData = self.underlyingData.dropFirst(2) | ||
|
|
||
| guard | ||
| octetHex.allSatisfy({ asciiDigits.contains($0) || asciiLowercases.contains($0) }), |
There was a problem hiding this comment.
This would be somewhere I'd write a little micro-extension method for this purpose, as we know that there are exactly two values and so we don't strictly need a loop.
|
|
||
| extension UInt8 { | ||
| /// Converts a hex value given in UTF8 to base 10. | ||
| fileprivate static func decodeHex<Bytes: RandomAccessCollection>(_ bytes: Bytes) -> Self? |
There was a problem hiding this comment.
This is also excessively generic I think. We only ever accept a single data type, and it's always two bytes long. It might actually be useful to express that as its own type, e.g:
struct EncodedHex {
var first: UInt8
var second: UInt8
init(_ slice: ArraySlice<UInt8>) {
precondition(slice.count == 2)
self.first = slice[slice.startIndex]
self.second = slice[slice.index(after: slice.startIndex)]
}
}
That lets us express the rest of this math in a much less branchy way, and with an avoidance of preconditions elsewhere.
| /// Converts a hex character given in UTF8 to its integer value. | ||
| private static func htoi(_ value: Self) -> Self? { | ||
| let charA = Self(UnicodeScalar("a").value) | ||
| let char0 = Self(UnicodeScalar("0").value) |
There was a problem hiding this comment.
Prefer the UInt8(ascii:) initializer.
| let char0 = Self(UnicodeScalar("0").value) | ||
|
|
||
| switch value { | ||
| case char0...char0 + 9: |
There was a problem hiding this comment.
These are a touch nicer using named values at both ends of the range.
| || (0x7F...).contains(byte) | ||
| { | ||
| self.data.append(asciiPercent) | ||
| self.data.append(contentsOf: String(byte, radix: 16, uppercase: false).utf8) |
There was a problem hiding this comment.
I think we can avoid constructing a String here, if we write the code out.
Lukasa
left a comment
Lukasa
left a comment
There was a problem hiding this comment.
Great, just a few quick notes and then we're good. Thanks for this!
| throw StructuredHeaderError.invalidDisplayString | ||
| } | ||
|
|
||
| let octetHex = EncodedHex(ArraySlice(self.underlyingData.prefix(2))) |
There was a problem hiding this comment.
I don't think the ArraySlice constructor should be necessary.
| let octetHex = EncodedHex(ArraySlice(self.underlyingData.prefix(2))) | |
| let octetHex = EncodedHex(self.underlyingData.prefix(2)) |
| try $0.withMemoryRebound(to: CChar.self) { | ||
| guard let baseAddress = $0.baseAddress else { | ||
| throw StructuredHeaderError.invalidDisplayString | ||
| } |
There was a problem hiding this comment.
You don't need this check: the base address will never be nil here.
Lukasa
left a comment
Lukasa
left a comment
There was a problem hiding this comment.
Cool, this LGTM. Nice one @clintonpi!
|
API breakage is expected, merging over it. |
3 participants
Motivation:
RFC 9651 added the Display String Structured Type.
Modifications:
Result:
The RawStructuredFieldValues module will support the Display String type.