Motivation
In machines with more complex network topologies it is possible for us
to have multiple possible NICs we might want to use for a request. Users
may wish to vary this on a per-request or even a per-client basis.
This control can typically be expressed by offering a local address to
bind to before making the connection attempt.
Modifications
Allow users to express a preferred local address at request or client
scope.
Make this part of the connection pool key.
Bind the local address when specified.
Test all of this.
Results
More capable clients.
Replaces all the foundation imports.
One issue is that `HTTPClient.init?(httpsURLWithSocketPath socketPath:
String, uri: String = "/")` uses `addingPercentEncoding()` from
Foundation. So instead, we use a pure Swift impl. that does the same.
We also need to disable default traits from `swift-configuration` to
prevent linking Foundation, because the `JSON` trait does that.
This also adds a linkage test to prevent regressions to CI.
Add a configuration option to retain the HTTP method and body receiving
301 or 302 responses.
Currently we automatically change the method to GET, and remove the
body, before following a 301 or 302. This is compliant with the fetch
specification: https://fetch.spec.whatwg.org/#http-redirect-fetch
However, it is useful to be able to override this behaviour and retain
the method and body.
Changes
- Add a new struct to encapsulate the (now 4) arguments of the follow
case of the redirect mode
- Add new options `retainHTTPMethodAndBodyOn301` and
`retainHTTPMethodAndBodyOn302`. Defaults to false because thats the
existing behaviour today
- When it is true, do not convert requests to GET after following a
redirect
- Note: this does not affect 307/308 (or any other) redirects. They
always preserve their method
---------
Co-authored-by: Fabian Fett <fabianfett@apple.com>
Swift log now has an InMemoryLogHandler. Lets depend on that instead of
having our own `CollectEverythingLogHandler`.
I've added an extension on top, to make it easier to create the logger
too
Result: less code
Motivation:
Now that strict concurrency has been adopted the AHC should avoid
regressing by treating all warnings as errors in CI.
Modifications:
- Treat warnings as errors in CI
Result:
Stricter CI
# Motivation
The NIO 2.78 release introduced a bunch of new warnings. These warnings
cause us a bunch of trouble, so we should fix them.
# Modifications
Mostly use a bunch of assumeIsolated() and syncOperations.
# Result
CI passes again.
Note that Swift 6 has _many_ more warnings than this, but we expect more
to come and we aren't using warnings-as-errors on that mode at the
moment. We'll be cleaning that up soon.
Enable MemberImportVisibility check on all targets. Use a standard
string header and footer to bracket the new block for ease of updating
in the future with scripts.
fixes#784
`writeChunks` had 3 bugs:
1. An actually wrong `UnsafeMutableTransferBox` -> removed that type
which should never be created
2. A loooong future chain (instead of one final promise) -> implemented
3. Potentially infinite recursion which lead to the crash in #784) ->
fixed too
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.
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:
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:
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:
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
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.
* Handle ResponseAccumulator not being able to buffer large response in memory
Check content length header for early exit
* Add test which currently hangs indefinitely
* Run `generate_linux_tests.rb` and `SwiftFormat`
* Print type and value if assert fails
* Run `generate_linux_tests.rb` and `SwiftFormat`
* Remove duplicate test due too merge conflict
* Validate that maxBodySize is positive
* Address review comments
SwiftNIO 2.42.0 has deprecated Lock and replaced it with a new NIOLock. This commit removes all uses of Lock and replaces them with NIOLock. Further, now, we must require SwiftNIO 2.42.0
* Fix thread leak in `FileDownloadDelegate`
* `SwiftFormat`
* Add a shared file IO thread pool per HTTPClient
* User bigger thread pool and initlize lazily during first file write
* thread pool is actually not used in tests
* Update documentation
* fix review comments
* make `fileIOThreadPool` internal
* Add test to verify that we actually share the same thread pool across all delegates for a given HTTPClient
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation
When receiving certain patterns of response body parts, we can end up
recursing almost indefinitely to deliver them to the application. This
can lead to crashes, so we might politely describe it as "sub-optimal".
Modifications
Keep track of our stack depth and avoid creating too many stack frames.
Added some unit tests.
Result
We no longer explode when handling bodies with lots of tiny parts.
Co-authored-by: David Nadoba <d_nadoba@apple.com>
Motivation
When users stream their bodies they may still want to send Connection:
close headers and terminate the connection early. This should work
properly.
Unfortunately it became clear that we didn't correctly pass the
information that the connection needed to be closed. As a result, we'd
inappropriately re-use the connection, potentially causing unnecessary
HTTP errors.
Modifications
Signal whether the connection needs to be closed when the final
connection action is to send .end.
Results
We behave better with streaming uploads.
Motivation
In some cases, the last thing that happens in a request-response pair is
that we send our HTTP/1.1 .end. This can happen when the peer has sent
an early response, before we have finished uploading our body. When it
does, we need to be diligent about cleaning up our connection state.
Unfortunately, there was an edge in the HTTP1ConnectionStateMachine that
processed .succeedRequest but that did not transition the state into
either .idle or .closing. That was an error, and needed to be fixed.
Modifications
Transition to .idle when we're returning
.succeedRequest(.sendRequestEnd).
Result
Fewer crashes
Motivation
It's totally acceptable for a HTTP server to respond before a request
upload has completed. If the response is an error, we should abort the
upload (and we do), but if the response is a 2xx we should probably just
finish the upload.
In this case it turns out we'll actually hit a crash when we attempt to
deliver an empty body message. his is no good!
Once that bug was fixed it revealed another: while we'd attempted to
account for this case, we hadn't tested it, and so it turns out that
shutdown would hang. As a result, I've also cleaned that up.
Modifications
- Tolerate empty circular buffers of bytes when streaming an upload.
- Notify the connection that the task is complete when we're done.
Result
Fewer crashes and hangs.
### Motivation
To land support for async/await, we need test utilities. We already have a `MockRequestExecutor`. Let's improve this to better handle on and off `EventLoop` request processing.
### Changes
- Move `MockRequestExecutor` into its own file
- Add blocking APIs to `MockRequestExecutor` when called from another thread
### Motivation
Our current swiftformat version does not support async/await. Since we want to add support for async/await we must update swiftformat or disable it. I tried my very best to keep the number of changes as small as possible. I assume we want to stick with the new 0.48.8 for some time.
### Changes
- Update swiftformat to 0.48.8
### Result
We can land async/await code.
### Motivation
Fixes#238 and #231.
### Changes
- Extracted the unclean shutdown test from `HTTPClientTests` into their own file `HTTPClientUncleanSSLConnectionShutdownTests`
- Copy and pasted @weissi great explanation from #238 into the test file
- Removed property `ignoreUncleanSSLShutdown` everywhere
### Result
`ignoreUncleanSSLShutdown` on `HTTPClient.Configuration` is deprecated and ignored.
Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
- Add `swift-nio-http2` as a dependency to the Package.swift (only used in the test target so far).
- Remove unused initialization options for `HTTPBin`
- The initialization options `ssl: Bool = false, compress: Bool = false, refusesConnections: Bool = false` move into a `Mode` enum. The mode might be `.http1_1(ssl: Bool, compress: Bool)`, `.http2(compress: Bool)`, or `.refuse`
- Added support for `http/2` (not used in any tests yet, however I verified the support with curl locally.)
- Nearly all channel pipeline modifications in `HTTPBin` are synchronous now.
- HTTPBin's request handler is configurable. This allows testing of more esoteric cases without having to create a new server every time. The default `HTTPBin` continues to use the `HTTPBinHandler`.
Updated:
NIO
NIOSSL
NIO Extras
NIOTS
Also fix TLSConfiguration.forClient() warnings by converting to TLSConfiguration.makeClientConfiguration(). Also the same for forServer().
Currently the DelayOnHeadDelegate test utility depends on correct timings. Right now, if a request is slower than 50ms in `testErrorAfterCloseWhileBackpressureExerted` the test will fail, since the backpressure promise is failed, before a head was received. This pr fixes this, by giving the user a callback, when the head was received.
Motivation:
Users of the HTTPClientResponseDelegate expect that the event loop
futures returned from didReceiveHead and didReceiveBodyPart can be used
to exert backpressure. To be fair to them, they somewhat can. However,
the TaskHandler has a bit of a misunderstanding about how NIO
backpressure works, and does not correctly manage the buffer of inbound
data.
The result of this misunderstanding is that multiple calls to
didReceiveBodyPart and didReceiveHead can be outstanding at once. This
would likely lead to severe bugs in most delegates, as they do not
expect it.
We should make things work the way delegate implementers believe it
works.
Modifications:
- Added a buffer to the TaskHandler to avoid delivering data that the
delegate is not ready for.
- Added a new "pending close" state that keeps track of a state where
the TaskHandler has received .end but not yet delivered it to the
delegate. This allows better error management.
- Added some more tests.
- Documented our backpressure commitments.
Result:
Better respect for backpressure.
Resolves#348
Motivation:
When we stream request body, current implementation expects that body
will finish streaming _before_ we start to receive response body parts.
This is not correct, reponse body parts can start to arrive before we
finish sending the request.
Modifications:
- Simplifies state machine, we only case about request being fully sent
to prevent sending body parts after .end, but response state machine
is mostly ignored and correct flow will be handled by NIOHTTP1
pipeline
- Adds HTTPEchoHandler, that replies to each response body part
- Adds bi-directional streaming test
Result:
Closes#327
Motivation:
Currently when either we or the server send Connection: close, we
correctly do not return that connection to the pool. However, we rely on
the server actually performing the connection closure: we never call
close() ourselves. This is unnecessarily optimistic: a server may
absolutely fail to close this connection. To protect our own file
descriptors, we should make sure that any connection we do not return
the pool is closed.
Modifications:
If we think a connection is closing when we release it, we now call
close() on it defensively.
Result:
We no longer leak connections when the server fails to close them.
Fixes#324.
* Add FileDownloadDelegate for simple file downloads
* Add testFileDownload to the allTests array
* Fix formatting
* Fix compatibility with Swift 5.0
* Add doc comments, update README.md
* Refine FileDownloadDelegate description in README
* Bump NIO version, remove weak self, cleanup test
* Fix formatting issues in a doc comment
* Create separate Progress struct, async open file
* Create an ad-hoc EventLoopGroup for opening a file
* Move file opening code to `didReceiveBodyPart`
* Fix linter error in FileDownloadDelegate.swift
* Fix wrong future assignment in FileDownloadDelegate
* Fix Swift 5.0 return statement compatibility
* Fix linter warning
* Fix Swift 5.0 return statement compatibility
* Remove redundant `write` function
* Add negative test case and separate testing endpoint
* Add missing testFileDownloadError to the manifest
Motivation:
Build warnings fail our builds.
Modifications:
- Update minimum NIOSSL version
- Remove the use of the now-unnecessary `try` statement.
Result:
No warnings!
Cory Benfield
Mads Odgaard
hamzahrmalik
George Barnett
Marc Prud'hommeaux
Rick Newton-Rogers
Johannes Weiss
Ian Anderson
Gustavo Cairo
Alastair Houghton
Peter Adams
brenno
carolinacass
David Nadoba
Fabian Fett
Karl
David Evans
Artem Redkin
Max Desiatov