Skip to content

Allow max reset stream frames to be configured #494

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Lukasa merged 1 commit into from
Feb 28, 2025
Merged

Allow max reset stream frames to be configured #494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions Sources/NIOHTTP2/ConnectionStateMachine/ConnectionStateMachine.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ struct HTTP2ConnectionStateMachine {
let role: ConnectionRole
var headerBlockValidation: ValidationState
var contentLengthValidation: ValidationState
var maxResetStreams: Int
}

/// The state required for a connection that has sent a connection preface.
Expand Down Expand Up @@ -98,7 +99,7 @@ struct HTTP2ConnectionStateMachine {
self.headerBlockValidation = idleState.headerBlockValidation
self.contentLengthValidation = idleState.contentLengthValidation
self.localSettings = settings
self.streamState = ConnectionStreamState()
self.streamState = ConnectionStreamState(maxResetStreams: idleState.maxResetStreams)

self.inboundFlowControlWindow = HTTP2FlowControlWindow(initialValue: settings.initialWindowSize)
self.outboundFlowControlWindow = HTTP2FlowControlWindow(
Expand Down Expand Up @@ -136,7 +137,7 @@ struct HTTP2ConnectionStateMachine {
self.headerBlockValidation = idleState.headerBlockValidation
self.contentLengthValidation = idleState.contentLengthValidation
self.remoteSettings = settings
self.streamState = ConnectionStreamState()
self.streamState = ConnectionStreamState(maxResetStreams: idleState.maxResetStreams)

self.inboundFlowControlWindow = HTTP2FlowControlWindow(
initialValue: HTTP2SettingsState.defaultInitialWindowSize
Expand Down Expand Up @@ -571,13 +572,15 @@ struct HTTP2ConnectionStateMachine {
init(
role: ConnectionRole,
headerBlockValidation: ValidationState = .enabled,
contentLengthValidation: ValidationState = .enabled
contentLengthValidation: ValidationState = .enabled,
maxResetStreams: Int = 32
) {
self.state = .idle(
.init(
role: role,
headerBlockValidation: headerBlockValidation,
contentLengthValidation: contentLengthValidation
contentLengthValidation: contentLengthValidation,
maxResetStreams: maxResetStreams
)
)
}
Expand Down
12 changes: 5 additions & 7 deletions Sources/NIOHTTP2/ConnectionStateMachine/ConnectionStreamsState.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,6 @@ struct ConnectionStreamState {
/// this case.
private var recentlyResetStreams: CircularBuffer<HTTP2StreamID>

/// The maximum number of reset streams we'll persist.
///
/// TODO (cory): Make this configurable!
private let maxResetStreams: Int = 32

/// The current number of streams that are active and that were initiated by the client.
private var clientStreamCount: UInt32 = 0

Expand Down Expand Up @@ -63,9 +58,12 @@ struct ConnectionStreamState {
Int(self.clientStreamCount) + Int(self.serverStreamCount)
}

init() {
/// Creates a new `ConnectionStreamState`.
///
/// - Parameter maxResetStreams: The maximum number of reset streams we'll persist.
init(maxResetStreams: Int) {
self.activeStreams = StreamMap()
self.recentlyResetStreams = CircularBuffer(initialCapacity: self.maxResetStreams)
self.recentlyResetStreams = CircularBuffer(initialCapacity: maxResetStreams)
}

/// Create stream state for a remotely pushed stream.
Expand Down
16 changes: 14 additions & 2 deletions Sources/NIOHTTP2/HTTP2ChannelHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler {

/// The default value for the maximum number of sequential CONTINUATION frames.
private static let defaultMaximumSequentialContinuationFrames: Int = 5
/// The default number of recently reset streams to track.
private static let defaultMaximumRecentlyResetFrames: Int = 32

/// The event loop on which this handler will do work.
@usableFromInline internal let _eventLoop: EventLoop?
Expand Down Expand Up @@ -233,6 +235,7 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler {
maximumSequentialEmptyDataFrames: 1,
maximumBufferedControlFrames: 10000,
maximumSequentialContinuationFrames: NIOHTTP2Handler.defaultMaximumSequentialContinuationFrames,
maximumRecentlyResetStreams: Self.defaultMaximumRecentlyResetFrames,
maximumResetFrameCount: 200,
resetFrameCounterWindow: .seconds(30),
maximumStreamErrorCount: 200,
Expand Down Expand Up @@ -269,6 +272,7 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler {
maximumSequentialEmptyDataFrames: maximumSequentialEmptyDataFrames,
maximumBufferedControlFrames: maximumBufferedControlFrames,
maximumSequentialContinuationFrames: NIOHTTP2Handler.defaultMaximumSequentialContinuationFrames,
maximumRecentlyResetStreams: Self.defaultMaximumRecentlyResetFrames,
maximumResetFrameCount: 200,
resetFrameCounterWindow: .seconds(30),
maximumStreamErrorCount: 200,
Expand Down Expand Up @@ -297,6 +301,7 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler {
maximumSequentialEmptyDataFrames: connectionConfiguration.maximumSequentialEmptyDataFrames,
maximumBufferedControlFrames: connectionConfiguration.maximumBufferedControlFrames,
maximumSequentialContinuationFrames: connectionConfiguration.maximumSequentialContinuationFrames,
maximumRecentlyResetStreams: connectionConfiguration.maximumRecentlyResetStreams,
maximumResetFrameCount: streamConfiguration.streamResetFrameRateLimit.maximumCount,
resetFrameCounterWindow: streamConfiguration.streamResetFrameRateLimit.windowLength,
maximumStreamErrorCount: streamConfiguration.streamErrorRateLimit.maximumCount,
Expand All @@ -313,6 +318,7 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler {
maximumSequentialEmptyDataFrames: Int,
maximumBufferedControlFrames: Int,
maximumSequentialContinuationFrames: Int,
maximumRecentlyResetStreams: Int,
maximumResetFrameCount: Int,
resetFrameCounterWindow: TimeAmount,
maximumStreamErrorCount: Int,
Expand All @@ -322,7 +328,8 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler {
self.stateMachine = HTTP2ConnectionStateMachine(
role: .init(mode),
headerBlockValidation: .init(headerBlockValidation),
contentLengthValidation: .init(contentLengthValidation)
contentLengthValidation: .init(contentLengthValidation),
maxResetStreams: maximumRecentlyResetStreams
)
self.mode = mode
self.initialSettings = initialSettings
Expand Down Expand Up @@ -371,6 +378,7 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler {
maximumBufferedControlFrames: Int = 10000,
maximumSequentialContinuationFrames: Int = NIOHTTP2Handler.defaultMaximumSequentialContinuationFrames,
tolerateImpossibleStateTransitionsInDebugMode: Bool = false,
maximumRecentlyResetStreams: Int = NIOHTTP2Handler.defaultMaximumRecentlyResetFrames,
maximumResetFrameCount: Int = 200,
resetFrameCounterWindow: TimeAmount = .seconds(30),
maximumStreamErrorCount: Int = 200,
Expand All @@ -379,7 +387,8 @@ public final class NIOHTTP2Handler: ChannelDuplexHandler {
self.stateMachine = HTTP2ConnectionStateMachine(
role: .init(mode),
headerBlockValidation: .init(headerBlockValidation),
contentLengthValidation: .init(contentLengthValidation)
contentLengthValidation: .init(contentLengthValidation),
maxResetStreams: maximumRecentlyResetStreams
)
self.mode = mode
self._eventLoop = nil
Expand Down Expand Up @@ -1320,6 +1329,7 @@ extension NIOHTTP2Handler {
maximumSequentialEmptyDataFrames: connectionConfiguration.maximumSequentialEmptyDataFrames,
maximumBufferedControlFrames: connectionConfiguration.maximumBufferedControlFrames,
maximumSequentialContinuationFrames: connectionConfiguration.maximumSequentialContinuationFrames,
maximumRecentlyResetStreams: connectionConfiguration.maximumRecentlyResetStreams,
maximumResetFrameCount: streamConfiguration.streamResetFrameRateLimit.maximumCount,
resetFrameCounterWindow: streamConfiguration.streamResetFrameRateLimit.windowLength,
maximumStreamErrorCount: streamConfiguration.streamErrorRateLimit.maximumCount,
Expand Down Expand Up @@ -1351,6 +1361,7 @@ extension NIOHTTP2Handler {
maximumSequentialEmptyDataFrames: connectionConfiguration.maximumSequentialEmptyDataFrames,
maximumBufferedControlFrames: connectionConfiguration.maximumBufferedControlFrames,
maximumSequentialContinuationFrames: connectionConfiguration.maximumSequentialContinuationFrames,
maximumRecentlyResetStreams: connectionConfiguration.maximumRecentlyResetStreams,
maximumResetFrameCount: streamConfiguration.streamResetFrameRateLimit.maximumCount,
resetFrameCounterWindow: streamConfiguration.streamResetFrameRateLimit.windowLength,
maximumStreamErrorCount: streamConfiguration.streamErrorRateLimit.maximumCount,
Expand All @@ -1374,6 +1385,7 @@ extension NIOHTTP2Handler {
public var maximumSequentialEmptyDataFrames: Int = 1
public var maximumBufferedControlFrames: Int = 10000
public var maximumSequentialContinuationFrames: Int = NIOHTTP2Handler.defaultMaximumSequentialContinuationFrames
public var maximumRecentlyResetStreams: Int = NIOHTTP2Handler.defaultMaximumRecentlyResetFrames
public init() {}
}

Expand Down
54 changes: 54 additions & 0 deletions Tests/NIOHTTP2Tests/ConnectionStateMachineTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7676,6 +7676,60 @@ class ConnectionStateMachineTests: XCTestCase {
self.client.receiveHeaders(streamID: streamOne, headers: responseHeaders, isEndStreamSet: false)
)
}

func testMaxRecentlyResetStreamFrames() {
// Internally a circular buffer is used to track recently reset streams which has its capacity
// reserved on init. This is rounded up to the next power of 2. However, in order to
// not grow beyond that size its effective capacity is less than the allocated capacity.
//
// Setting maxResetStreams to 64 means the effective capacity is 63.
let effectiveMaxResetStreams = 63
// Create one more stream than that to check that going beyond the limit causes an error.
let streamsToCreate = 64

self.client = HTTP2ConnectionStateMachine(role: .client, maxResetStreams: effectiveMaxResetStreams)
self.exchangePreamble()

let streamIDs = (0..<streamsToCreate).map { HTTP2StreamID($0 * 2 + 1) }
print(streamIDs.count)

// Open the streams; both peers know about them.
for streamID in streamIDs {
assertSucceeds(
self.client.sendHeaders(streamID: streamID, headers: Self.requestHeaders, isEndStreamSet: false)
)
assertSucceeds(
self.server.receiveHeaders(streamID: streamID, headers: Self.requestHeaders, isEndStreamSet: false)
)
}

// Have the client reset all streams.
for streamID in streamIDs {
assertSucceeds(self.client.sendRstStream(streamID: streamID, reason: .cancel))
}

// Have the server send response headers. The server allows it (it hasn't yet received RST_STREAM). The client ignores all
// but the last (which is one more than the limit).
for streamID in streamIDs {
assertSucceeds(
self.server.sendHeaders(streamID: streamID, headers: Self.responseHeaders, isEndStreamSet: false)
)
let result = self.client.receiveHeaders(
streamID: streamID,
headers: Self.responseHeaders,
isEndStreamSet: false
)

// The first stream results in a connection error, it was the first to be reset so the
// first to be forgotten about when the client sent the RST_STREAM frames.
if streamID == streamIDs.first {
assertConnectionError(type: .streamClosed, result)
} else {
assertIgnored(result)
}
}

}
}

extension HPACKHeaders {
Expand Down