- Fix a `fatalError("Invalid state: idle")` crash in
`HTTP1ConnectionStateMachine.demandMoreResponseBodyParts()` that occurs
when a response completes before the request body finishes uploading
- The root cause is that `self.request` was only nilled out inside the
write-completion callback for `.sendRequestEnd`, creating a window where
`demandResponseBodyStream` could still see the old request and call into
the state machine after it had already transitioned to .idle
- The fix nils out `self.request` synchronously when handling
`.sendRequestEnd` (before the write completes), and moves
`requestBodyStreamSent()` to fire after the final action rather than
before it
---------
Co-authored-by: George Barnett <gbarnett@apple.com>
Motivation:
The tests shouldn't be making sendability violations.
Modifications:
Fix the warnings
Result:
- No warnings
- Strict concurrency is adopted!
### Motivation:
In some cases we can crash because of a precondition failure when the
write timeout fires and we aren't in the running state. This can happen
for example if the connection is closed whilst the write timer is
active.
### Modifications:
* Remove the precondition and instead take no action if the timeout
fires outside of the running state. Instead we take a new `Action`,
`.noAction` when the timer fires.
* Clear write timeouts upon request completion. When a request completes
we have no use for the idle write timer, we clear the read timer and we
should clear the write one too.
### Result:
Fewer crashes.
The supplied tests fails without these changes and passes with either of them.
Migrate CI to use GitHub Actions.
### Motivation:
To migrate to GitHub actions and centralised infrastructure.
### Modifications:
Changes of note:
* Adopt swift-format using rules from SwiftNIO.
* Remove scripts and docker files which are no longer needed.
* Disabled warnings-as-errors on Swift 6.0 CI pipelines for now.
### Result:
Feature parity with old CI.
### Motivation
If the channel's writability changed to false just before we finished a
request, we currently run into a precondition.
### Changes
- Remove the precondition and handle the case appropiatly
### Result
A crash less.
### 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.
* Reproducer
* Refactor test case
* Refactor tests
* Remove debugging artefacts
* Fix typo
* Fix formatting
* Remove `promise?.succeed(())`
* Add test for HTTP2 request with large header
Motivation
We currently don't handle large headers well which trigger a channel writability change event.
Modification
Add failing (but currently skipped) tests which reproduces the issue
Result
We can reliably reproduce the large request header issue in an integration and unit test.
Note that the actual fix is not included to make reviewing easier and will come in a follow up PR.
* Remove logging
* Fix crash for large HTTP request headers
Fix crash for when sending HTTP request headers result in a channel writability change event
* Formatting and linux tests
* Formatting and linux tests
* Generate linux tests
* Use previous default max concurrent streams value of 10
* Fix crash if request is canceled after request header is send
* generate linux tests and run swift format
---------
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation
If we receive an early HTTP response, the last action on a HTTP/1.1
connection is to send the .end message. While we had an error handling
path in the code, it wasn't tested, and when executed it would end up
leaking the connection by failing to close it _or_ return it to the
pool.
This patch fixes the issue by appropriately terminating the connection
and adding a test.
Modifications
Add a test
Terminate the connection if sendEnd fails
Result
Fewer connection leaks
Same fix for HTTP/1 that landed for HTTP/2 in #558.
### Motivation
`HTTP1ClientChannelHandler` currently does not tolerate immediate write errors.
### Changes
Make `HTTP1ClientChannelHandler` resilient to failing writes.
### Result
Less crashes in AHC HTTP/1.
- a new `RequestOptions` struct was created, that can be used to set request specific options. It is required by the `HTTPExecutableRequest`
- Added support for `ignoreUncleanSSLShutdown` in the `HTTPRequestStateMachine` and the `HTTP1ConnectionStateMachine`. In http/2 `ignoreUncleanSSLShutdown` is always off.
Fabian Fett
Cory Benfield
George Barnett
Rick Newton-Rogers
aryan-25
Gustavo Cairo
David Nadoba