Skip to content

Respect maximumCount argument in request reader #44

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

Open
aryan-25 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Respect maximumCount argument in request reader #44

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
37 changes: 23 additions & 14 deletions Sources/HTTPServer/HTTPRequestConcludingAsyncReader.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,21 +71,30 @@ public struct HTTPRequestConcludingAsyncReader: ConcludingAsyncReader, ~Copyable
throw .first(error)
}

do {
switch requestPart {
case .head:
fatalError()
case .body(let element):
return try await body(Array(buffer: element).span)
case .end(let trailers):
self.state.wrapped.withLock { state in
state.trailers = trailers
state.finishedReading = true
}
return try await body(.init())
case .none:
return try await body(.init())
let readElement: ByteBuffer

switch requestPart {
case .head:
fatalError()
case .body(let element):
if let maximumCount, maximumCount < element.readableBytes {
throw .first(LimitExceeded())
}
Comment on lines +80 to +82
Copy link
Collaborator

@gjcairo gjcairo Jan 9, 2026

Choose a reason for hiding this comment

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

I'm actually unsure about this. I wonder if we should only return maximumCount bytes and keep the rest of the bytes in a local buffer until the next read call, or if we should throw. The only way to get out of this throwing scenario is by increasing the count, which sounds potentially problematic.


readElement = element
case .end(let trailers):
self.state.wrapped.withLock { state in
state.trailers = trailers
state.finishedReading = true
}

readElement = .init()
case .none:
readElement = .init()
}

do {
return try await body(Array(buffer: readElement).span)
} catch {
throw .second(error)
}
Expand Down
17 changes: 17 additions & 0 deletions Sources/HTTPServer/LimitExceeded.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift HTTP Server open source project
//
// Copyright (c) 2026 Apple Inc. and the Swift HTTP Server project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

/// Data available in a read buffer exceeded the caller-specified maximum count.
struct LimitExceeded: Error {
Copy link
Collaborator

@gjcairo gjcairo Jan 9, 2026

Choose a reason for hiding this comment

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

I'm not sure we need this based on the other comment but if we do, I think it makes more sense to have a namespace for this, either by nesting it inside the HTTPRequestConcludingAsyncReader or by having some other error namespace and nesting it there.

init() {}
}
Loading