Skip to content

Add Reassembly support for Swift IPv6#51

Open
agnosticdev wants to merge 2 commits into
mainfrom
agnosticdev/IPv6Reassembly
Open

Add Reassembly support for Swift IPv6#51
agnosticdev wants to merge 2 commits into
mainfrom
agnosticdev/IPv6Reassembly

Conversation

@agnosticdev

@agnosticdev agnosticdev commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

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 processInboundFrames it will take the fast path and do nothing with reassembly.

@agnosticdev
agnosticdev requested review from rnro and rpaulo July 14, 2026 23:26
Comment thread Sources/SwiftNetwork/Protocols/IPProtocol.swift Outdated
Comment thread Sources/SwiftNetwork/Protocols/IPProtocol.swift Outdated
var foundFragment = false

let result = Deserializer.deserialize(&frame, claim: false) { read throws(DeserializationError) in
try read.skip(4)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the first 4 bytes still be validated?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question: is this being validated anywhere else?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, the src and dest addresses were validated in processInboundFrames.

}
}
if !forceFlush {
if reassemblyState == nil {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could revert this and do if let reassemblyState = reassemblyState { and then you avoid reassemblyState?.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would not work because reassemblyState is a non-copyable struct and setting it locally would cause ownership issues.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 rpaulo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved pending minor comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants