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:
As an HTTP library, async-http-client should have authentication
support.
Modifications:
This adds a `setBasicAuth()` method to both HTTPClientRequest and
`HTTPClient.Request` and their related unit tests.
Result:
Library users will be able to leverage this method to use basic
authentication on their requests without implementing this in their own
projects.
Note: I also ran the tests (`swift test`) with the
`docker.io/library/swift:6.0-focal` and
`docker.io/library/swift:5.10.1-focal` to ensure linux compatibility.
Signed-off-by: Agam Dua <agam_dua@apple.com>
Multipath TCP (MPTCP) is a TCP extension allowing to enhance the
reliability of the network by using multiple interfaces. This extension
provides a seamless handover between interfaces in case of deterioration
of the connection on the original one. In the context of iOS and Mac OS
X, it could be really interesting to leverage the capabilities of MPTCP
as they could benefit from their multiple interfaces (ethernet + Wi-fi
for Mac OS X, Wi-fi + cellular for iOS).
This contribution introduces patches to HTTPClient.Configuration and
establishment of the Bootstraps. A supplementary field "enableMultipath"
was added to the configuration, allowing to request the use of MPTCP.
This flag is then used when creating the channels to configure the
client.
Note that in the future, it might also be potentially interesting to
offer more precise configuration options for MPTCP on MacOS, as the
Network framework allows also to select a type of service, instead of
just offering the option to create MPTCP connections. Currently, when
enabling MPTCP, only the Handover mode is used.
---------
Co-authored-by: Cory Benfield <lukasa@apple.com>
The Darwin module is slowly being split up, and as it gets further
along, it will stop importing some of the split-out modules like the one
for locale.h that provides newlocale() and other locale API. However,
there's a wrinkle that on platforms with xlocale, it's xlocale.h that
provides most of the POSIX locale.h functions and not locale.h, so
prefer the xlocale module when available.
### 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.
Since most of the servers now conform to http2, the change here updates
the behaviour of assuming the connection to be http2 and not http1 by
default. It will migrate to http1 if the server only supports http1.
One can set the `httpVersion` in `ClientConfiguration` to `.http1Only`
which will start with http1 instead of http2.
Additional Changes:
- Fixed an off by one error in the maximum additional general purpose
connection check
- Updated tests
---------
Co-authored-by: Ayush Garg <ayushgarg@apple.com>
Co-authored-by: David Nadoba <d_nadoba@apple.com>
Co-authored-by: Fabian Fett <fabianfett@apple.com>
### Motivation:
When a user wishes to make the connection pool create as many concurrent
connections as possible, a natural way to achieve this would be to set
`.max` to the `concurrentHTTP1ConnectionsPerHostSoftLimit` property.
```swift
HTTPClient.Configuration().connectionPool = .init(
idleTimeout: .hours(1),
concurrentHTTP1ConnectionsPerHostSoftLimit: .max
)
```
The `concurrentHTTP1ConnectionsPerHostSoftLimit` property is of type
`Int`. Setting it to `Int.max` leads to `Int.max` being passed as an
argument to `Array`s `.reserveCapacity(_:)` method, causing an OOM
issue.
Addresses Github Issue #751
### Modifications:
Capped the argument to `self.connections.reserveCapacity(_:)` to 1024 in
`HTTPConnectionPool.HTTP1Connections`
### Result:
Users can now set the `concurrentHTTP1ConnectionsPerHostSoftLimit`
property to `.max` without causing an OOM issue.
### 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.
* Make ConnectionPool's `retryConnectionEstablishment` public
* Unified tests to consistently use `enableFastFailureModeForTesting()`
* Add `retryConnectionEstablishment` as optional parameter to the initializer of `HTTPClient.Configuration.ConnectionPool`
* Reverted change to initializer to prevent API stability breakage
* Add parameterless initializer for `HTTPClient.Configuration.ConnectionPool`
* Moved default values for `HTTPClient.Configuration.ConnectionPool` to the property declarations, so they only have to be specified at one point
* Removed superfluous spaces
Co-authored-by: Cory Benfield <lukasa@apple.com>
* Re-added missing line break
---------
Co-authored-by: Cory Benfield <lukasa@apple.com>
It was previously accidentally moved to a different test, which does not need it
Async tests need the guard for macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0
Motivation:
We would like to make this work for Musl so that we can build fully
statically linked binaries that use AsyncHTTPClient.
Modifications:
Define `_GNU_SOURCE` as a compiler argument; doing it in a source file
doesn't work properly with modular headers.
Add imports of `Musl` in appropriate places.
`Musl` doesn't have `strptime_l`, so avoid using that.
Result:
async-http-client will build for Musl.
## Motivation
Some of the test code was missing availability guards for Apple platforms, resulting in build failures for these platforms, e.g.
```
error: 'AsyncSequence' is only available in iOS 13.0 or newer
```
## Modifications
Add missing availability guards. I've tried to keep them as scoped as possible.
## Result
Tests can now build for run on iOS and other Apple platforms.
Motivation:
The above test has been seen to fail with port already in use.
The test assumes that a port can be bound to twice in a row.
It's possible that another process takes the port between the two
binds - this can be stopped by binding a second time before stopping
the first.
Modifications:
Allow port reuse and reorder the test to bind a second time before
releasing the first.
Result:
Test should no longer be flaky.
Motivation:
Now that Swift 5.9 is GM we should update the supported versions and
remove 5.6
Modifications:
* Update `Package.swift`
* Remove `#if swift(>=5.7)` guards
* Delete the 5.6 docker compose file and make a 5.10 one
* Update docs
Result:
Remove support for Swift 5.6, add 5.10
* ChunkedCollection
* Use swift-algorithms
* SwiftFormat
* test chunking
* add documentation
* SwiftFormat
* fix old swift versions
* fix older swift versions
second attempt
* fix old swift versions
third attempt
* fix old swift versions
fourth attempt
* update documentation
* Add unit tests for NWWaitingHandler
Motivation:
Closes#589. Since we already have a public initializer for
NIOTSNetworkEvents.WaitingForConnectivity, we should add unit tests for
the handler now that it's straightforward.
Modifications:
The tests are in their own file `Tests/NWWaitingHandlerTests.swift`.
* Bump swift-nio-transport-services to 1.13.0
* SwiftFormat@0.48.8
* Apply suggestions from code review
Co-authored-by: David Nadoba <dnadoba@gmail.com>
---------
Co-authored-by: David Nadoba <dnadoba@gmail.com>
Motivation
URL parsing has changed in macOS Sonoma and associated releases. Our
tests were reliant on the old behaviour. The behaviour is controlled
by the OS on which the program was linked, which makes it impossible
for us to programmatically work out which result we should see.
The affected tests are not actually useful, we don't really care how
the URLs parse, so we can safely just remove them.
Modifications
Remove the affected tests.
Result
Tests pass!
We have seen that the `RequestBagTests.testCancelFailsTaskAfterRequestIsSent` can timeout the CI. To better understand where it fails we can add a timeout that will throw with a timeout error and fail the error check below it. This will make will allow us in the future to better analyse the failure and will no longer timeout the CI.
* save progress
* Replace `TransactionBody` with `NIOAsyncSequenceProducer`
* test
* revert unnesscary changes
* Add end-to-end test
which currently fails because of https://github.com/apple/swift-nio/issues/2398
* soundness
* Use latest swift-nio release
* throw CancellationError on task cancelation
* Fix Swift 5.5 & 5.6
Motivation:
Sometimes it can be helpful to limit the number of times a connection
can be used before discarding it. AHC has no such support for this at
the moment.
Modifications:
- Add a `maximumUsesPerConnection` configuration option which defaults
to `nil` (i.e. no limit).
- For HTTP1 we count down uses in the state machine and close the
connection if it hits zero.
- For HTTP2, each use maps to a stream so we count down remaining uses
in the state machine which we combine with max concurrent streams to
limit how many streams are available per connection. We also count
remaining uses in the HTTP2 idle handler: we treat no remaining uses
as receiving a GOAWAY frame and notify the pool which then drains the
streams and replaces the connection.
Result:
Users can control how many times each connection can be used.
Motivation:
not accumulate too many bytes
Modifications:
Implementing collect function to use NIOCore version to prevent overflowing
Co-authored-by: Cory Benfield <lukasa@apple.com>
Sometimes it can be useful to connect to one host e.g. `x.example.com` but request and validate the certificate chain as if we would connect to `y.example.com`. This is what this PR adds support for by adding a `dnsOverride` configuration to `HTTPClient.Configuration`. This is similar to curls `—resolve-to` option but only allows overriding host and not ports for now.
Motivation
Task.wait() is a convenience method to avoid needing to wait for the
response future. This has had the effect of "laundering" a noasync
warning from EventLoopFuture.wait(), hiding it within a purely sync call
that may itself be used in an async context.
We should discourage using these and prefer using .get() instead.
Modifications
Mark Task.wait() noasync.
Add Task.get() for backward compatibility.
Result
Safer migration to Swift concurrency
* 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
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.
* Add Swift 5.8 CI
* Update nightly CI to Ubuntu 22.04
* Fix warnings in test target with latest NIO release
Replace calls to `.wait()` with calls to `.get()`
Motivation
Allowing arbitrary data in outbound header field values allows for the
possibility that users of AHC will accidentally pass untrusted data into
those values. That untrusted data can substantially alter the parsing
and content of the HTTP requests, which is extremely dangerous. The
result of this is vulnerability to CRLF injection.
Modifications
Add validation of outbound header field values.
Result
No longer vulnerable to CRLF injection
Rick Newton-RogersandGitHub
Agam DuaandGitHub
Ian AndersonandGitHub
Fabian FettandGitHub
aryan-25andGitHub
Gustavo CairoandGitHub
hamzahrmalikandGitHub
hamzahrmalikandGitHub
Alastair HoughtonandGitHub
Si BeaumontandGitHub
Peter AdamsandGitHub
Rick Newton-RogersandGitHub
brennoandGitHub
George BarnettandGitHub
