Add Reassembly support for Swift IPv6#51
Conversation
| var foundFragment = false | ||
|
|
||
| let result = Deserializer.deserialize(&frame, claim: false) { read throws(DeserializationError) in | ||
| try read.skip(4) |
There was a problem hiding this comment.
Shouldn't the first 4 bytes still be validated?
There was a problem hiding this comment.
This would be redundant because the version / flow label was already validated in processInboundFrames, that is why its skipped here when the frame is parsed from the reassembledState.
| try read.uint16NetworkByteOrder(&payloadLength) | ||
| try read.uint8(&firstProto) | ||
| try read.skip(1) | ||
| try read.skip(32) // src and dst addresses |
There was a problem hiding this comment.
Same question: is this being validated anywhere else?
There was a problem hiding this comment.
Same here, the src and dest addresses were validated in processInboundFrames.
| } | ||
| } | ||
| if !forceFlush { | ||
| if reassemblyState == nil { |
There was a problem hiding this comment.
You could revert this and do if let reassemblyState = reassemblyState { and then you avoid reassemblyState?.
There was a problem hiding this comment.
This would not work because reassemblyState is a non-copyable struct and setting it locally would cause ownership issues.
There was a problem hiding this comment.
You can do it with this sort of trick:
switch reassemblyState {
case .some(let state):
// do stuff with state which is now non nil reassemblyState
case .none:
// nil reassemblyState
}
Or you can even do if .case(let state) = reassemblyState tricks.
rpaulo
left a comment
There was a problem hiding this comment.
Approved pending minor comments.
This change adds reassembly support for inbound IPv6 fragments.
This change only supports inbound fragmentation and reassembly. Outbound fragmentation will come in a later change.
This change takes a similar approach to IPv4 reassembly support, if no fragments are detected in
processInboundFramesit will take the fast path and do nothing with reassembly.