Skip to content

Commit 4316eca

Browse files
aryan-25aryan-25
authored
Add support for request body to be larger than 2GB on 32-bit devices (#746)
### Motivation: - The properties that store the request body length and the cumulative number of bytes sent as part of a request are of type `Int`. - On 32-bit devices, when sending requests larger than `Int32.max`, these properties overflow and cause a crash. - To solve this problem, the properties should use the explicit `Int64` type. ### Modifications: - Changed the type of the `known` field of the `RequestBodyLength` enum to `Int64`. - Changed the type of `expectedBodyLength` and `sentBodyBytes` in `HTTPRequestStateMachine` to `Int64?` and `Int64` respectively. - Deprecated the `public var length: Int?` property of `HTTPClient.Body` and backed it with a new property: `contentLength: Int64?` - Added a new initializer and "overloaded" the `stream` function in `HTTPClient.Body` to work with the new `contentLength` property. - **Note:** The newly added `stream` function has different parameter names (`length` -> `contentLength` and `stream` -> `bodyStream`) to avoid ambiguity problems. - Added a test case that streams a 3GB request -- verified this fails with the types of the properties set explicitly to `Int32`. ### Result: - 32-bit devices can send requests larger than 2GB without integer overflow issues.
1 parent e27aef4 commit 4316eca

17 files changed

+208
-110
lines changed

‎Sources/AsyncHTTPClient/AsyncAwait/HTTPClientRequest+Prepared.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ extension RequestBodyLength {
9595
case .none:
9696
self = .known(0)
9797
case .byteBuffer(let buffer):
98-
self = .known(buffer.readableBytes)
98+
self = .known(Int64(buffer.readableBytes))
9999
case .sequence(let length, _, _), .asyncSequence(let length, _):
100100
self = length
101101
}

‎Sources/AsyncHTTPClient/AsyncAwait/HTTPClientRequest.swift‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ extension HTTPClientRequest.Body {
125125
public static func bytes<Bytes: RandomAccessCollection & Sendable>(
126126
_ bytes: Bytes
127127
) -> Self where Bytes.Element == UInt8 {
128-
self.bytes(bytes, length: .known(bytes.count))
128+
self.bytes(bytes, length: .known(Int64(bytes.count)))
129129
}
130130

131131
/// Create an ``HTTPClientRequest/Body-swift.struct`` from a `Sequence` of bytes.
@@ -140,7 +140,7 @@ extension HTTPClientRequest.Body {
140140
///
141141
/// Caution should be taken with this method to ensure that the `length` is correct. Incorrect lengths
142142
/// will cause unnecessary runtime failures. Setting `length` to ``Length/unknown`` will trigger the upload
143-
/// to use `chunked` `Transfer-Encoding`, while using ``Length/known(_:)`` will use `Content-Length`.
143+
/// to use `chunked` `Transfer-Encoding`, while using ``Length/known(_:)-9q0ge`` will use `Content-Length`.
144144
///
145145
/// - parameters:
146146
/// - bytes: The bytes of the request body.
@@ -225,7 +225,7 @@ extension HTTPClientRequest.Body {
225225
///
226226
/// Caution should be taken with this method to ensure that the `length` is correct. Incorrect lengths
227227
/// will cause unnecessary runtime failures. Setting `length` to ``Length/unknown`` will trigger the upload
228-
/// to use `chunked` `Transfer-Encoding`, while using ``Length/known(_:)`` will use `Content-Length`.
228+
/// to use `chunked` `Transfer-Encoding`, while using ``Length/known(_:)-9q0ge`` will use `Content-Length`.
229229
///
230230
/// - parameters:
231231
/// - bytes: The bytes of the request body.
@@ -265,7 +265,7 @@ extension HTTPClientRequest.Body {
265265
///
266266
/// Caution should be taken with this method to ensure that the `length` is correct. Incorrect lengths
267267
/// will cause unnecessary runtime failures. Setting `length` to ``Length/unknown`` will trigger the upload
268-
/// to use `chunked` `Transfer-Encoding`, while using ``Length/known(_:)`` will use `Content-Length`.
268+
/// to use `chunked` `Transfer-Encoding`, while using ``Length/known(_:)-9q0ge`` will use `Content-Length`.
269269
///
270270
/// - parameters:
271271
/// - sequenceOfBytes: The bytes of the request body.
@@ -293,7 +293,7 @@ extension HTTPClientRequest.Body {
293293
///
294294
/// Caution should be taken with this method to ensure that the `length` is correct. Incorrect lengths
295295
/// will cause unnecessary runtime failures. Setting `length` to ``Length/unknown`` will trigger the upload
296-
/// to use `chunked` `Transfer-Encoding`, while using ``Length/known(_:)`` will use `Content-Length`.
296+
/// to use `chunked` `Transfer-Encoding`, while using ``Length/known(_:)-9q0ge`` will use `Content-Length`.
297297
///
298298
/// - parameters:
299299
/// - bytes: The bytes of the request body.
@@ -341,7 +341,13 @@ extension HTTPClientRequest.Body {
341341
public static let unknown: Self = .init(storage: .unknown)
342342

343343
/// The size of the request body is known and exactly `count` bytes
344+
@available(*, deprecated, message: "Use `known(_ count: Int64)` with an explicit Int64 argument instead")
344345
public static func known(_ count: Int) -> Self {
346+
.init(storage: .known(Int64(count)))
347+
}
348+
349+
/// The size of the request body is known and exactly `count` bytes
350+
public static func known(_ count: Int64) -> Self {
345351
.init(storage: .known(count))
346352
}
347353

‎Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ struct HTTPRequestStateMachine {
5858
/// The request is streaming its request body. `expectedBodyLength` has a value, if the request header contained
5959
/// a `"content-length"` header field. If the request header contained a `"transfer-encoding" = "chunked"`
6060
/// header field, the `expectedBodyLength` is `nil`.
61-
case streaming(expectedBodyLength: Int?, sentBodyBytes: Int, producer: ProducerControlState)
61+
case streaming(expectedBodyLength: Int64?, sentBodyBytes: Int64, producer: ProducerControlState)
6262
/// The request has sent its request body and end.
6363
case endSent
6464
}
@@ -308,13 +308,13 @@ struct HTTPRequestStateMachine {
308308
// pause. The reason for this is as follows: There might be thread synchronization
309309
// situations in which the producer might not have received the plea to pause yet.
310310

311-
if let expected = expectedBodyLength, sentBodyBytes + part.readableBytes > expected {
311+
if let expected = expectedBodyLength, sentBodyBytes + Int64(part.readableBytes) > expected {
312312
let error = HTTPClientError.bodyLengthMismatch
313313
self.state = .failed(error)
314314
return .failRequest(error, .close(promise))
315315
}
316316

317-
sentBodyBytes += part.readableBytes
317+
sentBodyBytes += Int64(part.readableBytes)
318318

319319
let requestState: RequestState = .streaming(
320320
expectedBodyLength: expectedBodyLength,
@@ -768,7 +768,7 @@ struct HTTPRequestStateMachine {
768768
}
769769

770770
extension RequestFramingMetadata.Body {
771-
var expectedLength: Int? {
771+
var expectedLength: Int64? {
772772
switch self {
773773
case .fixedSize(let length): return length
774774
case .stream: return nil

‎Sources/AsyncHTTPClient/ConnectionPool/RequestBodyLength.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,5 @@ internal enum RequestBodyLength: Hashable, Sendable {
2020
/// size of the request body is not known before starting the request
2121
case unknown
2222
/// size of the request body is fixed and exactly `count` bytes
23-
case known(_ count: Int)
23+
case known(_ count: Int64)
2424
}

‎Sources/AsyncHTTPClient/ConnectionPool/RequestFramingMetadata.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
struct RequestFramingMetadata: Hashable {
1616
enum Body: Hashable {
1717
case stream
18-
case fixedSize(Int)
18+
case fixedSize(Int64)
1919
}
2020

2121
var connectionClose: Bool

‎Sources/AsyncHTTPClient/HTTPHandler.swift‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,16 +70,28 @@ extension HTTPClient {
7070

7171
/// Body size. If nil,`Transfer-Encoding` will automatically be set to `chunked`. Otherwise a `Content-Length`
7272
/// header is set with the given `length`.
73-
public var length: Int?
73+
@available(*, deprecated, renamed: "contentLength")
74+
public var length: Int? {
75+
get {
76+
self.contentLength.flatMap { Int($0) }
77+
}
78+
set {
79+
self.contentLength = newValue.flatMap { Int64($0) }
80+
}
81+
}
82+
83+
/// Body size. If nil,`Transfer-Encoding` will automatically be set to `chunked`. Otherwise a `Content-Length`
84+
/// header is set with the given `contentLength`.
85+
public var contentLength: Int64?
7486

7587
/// Body chunk provider.
7688
public var stream: @Sendable (StreamWriter) -> EventLoopFuture<Void>
7789

7890
@usableFromInline typealias StreamCallback = @Sendable (StreamWriter) -> EventLoopFuture<Void>
7991

8092
@inlinable
81-
init(length: Int?, stream: @escaping StreamCallback) {
82-
self.length = length
93+
init(contentLength: Int64?, stream: @escaping StreamCallback) {
94+
self.contentLength = contentLength.flatMap { $0 }
8395
self.stream = stream
8496
}
8597

@@ -88,7 +100,7 @@ extension HTTPClient {
88100
/// - parameters:
89101
/// - buffer: Body `ByteBuffer` representation.
90102
public static func byteBuffer(_ buffer: ByteBuffer) -> Body {
91-
return Body(length: buffer.readableBytes) { writer in
103+
return Body(contentLength: Int64(buffer.readableBytes)) { writer in
92104
writer.write(.byteBuffer(buffer))
93105
}
94106
}
@@ -100,8 +112,19 @@ extension HTTPClient {
100112
/// header is set with the given `length`.
101113
/// - stream: Body chunk provider.
102114
@preconcurrency
115+
@available(*, deprecated, renamed: "stream(contentLength:bodyStream:)")
103116
public static func stream(length: Int? = nil, _ stream: @Sendable @escaping (StreamWriter) -> EventLoopFuture<Void>) -> Body {
104-
return Body(length: length, stream: stream)
117+
return Body(contentLength: length.flatMap { Int64($0) }, stream: stream)
118+
}
119+
120+
/// Create and stream body using ``StreamWriter``.
121+
///
122+
/// - parameters:
123+
/// - contentLength: Body size. If nil, `Transfer-Encoding` will automatically be set to `chunked`. Otherwise a `Content-Length`
124+
/// header is set with the given `contentLength`.
125+
/// - bodyStream: Body chunk provider.
126+
public static func stream(contentLength: Int64? = nil, bodyStream: @Sendable @escaping (StreamWriter) -> EventLoopFuture<Void>) -> Body {
127+
return Body(contentLength: contentLength, stream: bodyStream)
105128
}
106129

107130
/// Create and stream body using a collection of bytes.
@@ -111,7 +134,7 @@ extension HTTPClient {
111134
@preconcurrency
112135
@inlinable
113136
public static func bytes<Bytes>(_ bytes: Bytes) -> Body where Bytes: RandomAccessCollection, Bytes: Sendable, Bytes.Element == UInt8 {
114-
return Body(length: bytes.count) { writer in
137+
return Body(contentLength: Int64(bytes.count)) { writer in
115138
if bytes.count <= bagOfBytesToByteBufferConversionChunkSize {
116139
return writer.write(.byteBuffer(ByteBuffer(bytes: bytes)))
117140
} else {
@@ -125,7 +148,7 @@ extension HTTPClient {
125148
/// - parameters:
126149
/// - string: Body `String` representation.
127150
public static func string(_ string: String) -> Body {
128-
return Body(length: string.utf8.count) { writer in
151+
return Body(contentLength: Int64(string.utf8.count)) { writer in
129152
if string.utf8.count <= bagOfBytesToByteBufferConversionChunkSize {
130153
return writer.write(.byteBuffer(ByteBuffer(string: string)))
131154
} else {
@@ -858,7 +881,7 @@ extension RequestBodyLength {
858881
self = .known(0)
859882
return
860883
}
861-
guard let length = body.length else {
884+
guard let length = body.contentLength else {
862885
self = .unknown
863886
return
864887
}

‎Tests/AsyncHTTPClientTests/AsyncAwaitEndToEndTests.swift‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,62 @@ final class AsyncAwaitEndToEndTests: XCTestCase {
184184
}
185185
}
186186

187+
struct AsyncSequenceByteBufferGenerator: AsyncSequence, Sendable, AsyncIteratorProtocol {
188+
typealias Element = ByteBuffer
189+
190+
let chunkSize: Int
191+
let totalChunks: Int
192+
let buffer: ByteBuffer
193+
var chunksGenerated: Int = 0
194+
195+
init(chunkSize: Int, totalChunks: Int) {
196+
self.chunkSize = chunkSize
197+
self.totalChunks = totalChunks
198+
self.buffer = ByteBuffer(repeating: 1, count: self.chunkSize)
199+
}
200+
201+
mutating func next() async throws -> ByteBuffer? {
202+
guard self.chunksGenerated < self.totalChunks else { return nil }
203+
204+
self.chunksGenerated += 1
205+
return self.buffer
206+
}
207+
208+
func makeAsyncIterator() -> AsyncSequenceByteBufferGenerator {
209+
return self
210+
}
211+
}
212+
213+
func testEchoStreamThatHas3GBInTotal() async throws {
214+
let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1)
215+
defer { XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully()) }
216+
let bin = HTTPBin(.http1_1()) { _ in HTTPEchoHandler() }
217+
defer { XCTAssertNoThrow(try bin.shutdown()) }
218+
219+
let client: HTTPClient = makeDefaultHTTPClient(eventLoopGroupProvider: .shared(eventLoopGroup))
220+
defer { XCTAssertNoThrow(try client.syncShutdown()) }
221+
222+
let logger = Logger(label: "HTTPClient", factory: StreamLogHandler.standardOutput(label:))
223+
224+
var request = HTTPClientRequest(url: "http://localhost:\(bin.port)/")
225+
request.method = .POST
226+
227+
let sequence = AsyncSequenceByteBufferGenerator(
228+
chunkSize: 4_194_304, // 4MB chunk
229+
totalChunks: 768 // Total = 3GB
230+
)
231+
request.body = .stream(sequence, length: .unknown)
232+
233+
let response: HTTPClientResponse = try await client.execute(request, deadline: .now() + .seconds(30), logger: logger)
234+
XCTAssertEqual(response.headers["content-length"], [])
235+
236+
var receivedBytes: Int64 = 0
237+
for try await part in response.body {
238+
receivedBytes += Int64(part.readableBytes)
239+
}
240+
XCTAssertEqual(receivedBytes, 3_221_225_472) // 3GB
241+
}
242+
187243
func testPostWithAsyncSequenceOfByteBuffers() {
188244
XCTAsyncTest {
189245
let bin = HTTPBin(.http2(compress: false)) { _ in HTTPEchoHandler() }

‎Tests/AsyncHTTPClientTests/HTTP1ClientChannelHandlerTests.swift‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ class HTTP1ClientChannelHandlerTests: XCTestCase {
113113
guard let testUtils = maybeTestUtils else { return XCTFail("Expected connection setup works") }
114114

115115
var maybeRequest: HTTPClient.Request?
116-
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(length: 100) { writer in
116+
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(contentLength: 100) { writer in
117117
testWriter.start(writer: writer)
118118
}))
119119
guard let request = maybeRequest else { return XCTFail("Expected to be able to create a request") }
@@ -345,7 +345,7 @@ class HTTP1ClientChannelHandlerTests: XCTestCase {
345345
guard let testUtils = maybeTestUtils else { return XCTFail("Expected connection setup works") }
346346

347347
var maybeRequest: HTTPClient.Request?
348-
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(length: 10) { writer in
348+
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(contentLength: 10) { writer in
349349
// Advance time by more than the idle write timeout (that's 1 millisecond) to trigger the timeout.
350350
embedded.embeddedEventLoop.advanceTime(by: .milliseconds(2))
351351
return testWriter.start(writer: writer)
@@ -384,7 +384,7 @@ class HTTP1ClientChannelHandlerTests: XCTestCase {
384384
guard let testUtils = maybeTestUtils else { return XCTFail("Expected connection setup works") }
385385

386386
var maybeRequest: HTTPClient.Request?
387-
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(length: 10) { writer in
387+
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(contentLength: 10) { writer in
388388
embedded.isWritable = false
389389
embedded.pipeline.fireChannelWritabilityChanged()
390390
// This should not trigger any errors or timeouts, because the timer isn't running
@@ -432,7 +432,7 @@ class HTTP1ClientChannelHandlerTests: XCTestCase {
432432
guard let testUtils = maybeTestUtils else { return XCTFail("Expected connection setup works") }
433433

434434
var maybeRequest: HTTPClient.Request?
435-
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(length: 2) { writer in
435+
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(contentLength: 2) { writer in
436436
return testWriter.start(writer: writer, expectedErrors: [HTTPClientError.cancelled])
437437
}))
438438
guard let request = maybeRequest else { return XCTFail("Expected to be able to create a request") }
@@ -595,7 +595,7 @@ class HTTP1ClientChannelHandlerTests: XCTestCase {
595595
guard let testUtils = maybeTestUtils else { return XCTFail("Expected connection setup works") }
596596

597597
var maybeRequest: HTTPClient.Request?
598-
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(length: 10) { writer in
598+
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(contentLength: 10) { writer in
599599
testWriter.start(writer: writer)
600600
}))
601601
guard let request = maybeRequest else { return XCTFail("Expected to be able to create a request") }

‎Tests/AsyncHTTPClientTests/HTTP1ConnectionTests.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ class HTTP1ConnectionTests: XCTestCase {
116116
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(
117117
url: "http://localhost/hello/swift",
118118
method: .POST,
119-
body: .stream(length: 4) { writer -> EventLoopFuture<Void> in
120-
func recursive(count: UInt8, promise: EventLoopPromise<Void>) {
119+
body: .stream(contentLength: 4) { writer -> EventLoopFuture<Void> in
120+
@Sendable func recursive(count: UInt8, promise: EventLoopPromise<Void>) {
121121
guard count < 4 else {
122122
return promise.succeed(())
123123
}

‎Tests/AsyncHTTPClientTests/HTTP2ClientRequestHandlerTests.swift‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ class HTTP2ClientRequestHandlerTests: XCTestCase {
115115
let testWriter = TestBackpressureWriter(eventLoop: embedded.eventLoop, parts: 50)
116116

117117
var maybeRequest: HTTPClient.Request?
118-
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(length: 100) { writer in
118+
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(contentLength: 100) { writer in
119119
testWriter.start(writer: writer)
120120
}))
121121
guard let request = maybeRequest else { return XCTFail("Expected to be able to create a request") }
@@ -295,7 +295,7 @@ class HTTP2ClientRequestHandlerTests: XCTestCase {
295295

296296
let testWriter = TestBackpressureWriter(eventLoop: embedded.eventLoop, parts: 5)
297297
var maybeRequest: HTTPClient.Request?
298-
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(length: 10) { writer in
298+
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(contentLength: 10) { writer in
299299
// Advance time by more than the idle write timeout (that's 1 millisecond) to trigger the timeout.
300300
embedded.embeddedEventLoop.advanceTime(by: .milliseconds(2))
301301
return testWriter.start(writer: writer)
@@ -335,7 +335,7 @@ class HTTP2ClientRequestHandlerTests: XCTestCase {
335335

336336
let testWriter = TestBackpressureWriter(eventLoop: embedded.eventLoop, parts: 5)
337337
var maybeRequest: HTTPClient.Request?
338-
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(length: 10) { writer in
338+
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(contentLength: 10) { writer in
339339
embedded.isWritable = false
340340
embedded.pipeline.fireChannelWritabilityChanged()
341341
// This should not trigger any errors or timeouts, because the timer isn't running
@@ -385,7 +385,7 @@ class HTTP2ClientRequestHandlerTests: XCTestCase {
385385

386386
let testWriter = TestBackpressureWriter(eventLoop: embedded.eventLoop, parts: 5)
387387
var maybeRequest: HTTPClient.Request?
388-
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(length: 2) { writer in
388+
XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "http://localhost/", method: .POST, body: .stream(contentLength: 2) { writer in
389389
return testWriter.start(writer: writer, expectedErrors: [HTTPClientError.cancelled])
390390
}))
391391
guard let request = maybeRequest else { return XCTFail("Expected to be able to create a request") }

0 commit comments

Comments
 (0)