Motivation:
TaskHandler unconditionally releases it's connection on error,
this can lead to double release. This issue actually indicates
a more general issue where handler continues to handle errors
even after its state is `.endOrError`. We need to fix this by
ignoring all subsequent errors.
Modifications:
1. Check state before calling out delegate and pool
2. Replace all error callouts with call to `errorCaught`
Result:
Fixes#294
Motivation:
Build warnings fail our builds.
Modifications:
- Update minimum NIOSSL version
- Remove the use of the now-unnecessary `try` statement.
Result:
No warnings!
Motivation:
Streams length parameter is optional to allow cases were stream length is not known in advance, but we do not support this in request validation. This PR aims to address that.
Modifications:
Modifies request validation to default to chunked encoding if body length is zero or to passed in content-length header
Adds a test
Result:
Closes#218
* Added tests that ensure redirects to unix socket paths from a regular HTTP server are disallowed.
Motivation:
Currently, redirects to any supported URL scheme will always be allowed, despite code being in place to seemingly prevent it. See #230.
Modifications:
- Added a method to HTTPBin to redirect to the specified target.
- Added failing tests that perform redirects from a regular server to a socket-based server and vice versa.
Result:
Failing tests that show that the existing redirect checks were inadequate.
* Fixed an issue where redirects to socket path-based servers from any server was always allowed.
Motivation:
An arbitrary HTTP(S) server should not be able to trigger redirects, and thus activity, to a local socket-path based server, though the opposite may be a valid scenario. Currently, requests in either direction are allowed since the checks don't actually check the destination scheme.
Modifications:
- Refactored `hostSchemes`/`unixSchemes` to `hostRestrictedSchemes`/`allSupportedSchemes`, which better describes what they do.
- Refactored `Request.supports()` to `Request.supportsRedirects(to:)` since it is only used by Redirects now.
- Check the destination URL's scheme rather than the current URL's scheme when validating a redirect.
Result:
Closes#230
Co-authored-by: Artem Redkin <artem@redkin.me>
`redirectConfiguration` can't default to `false` as it's not a boolean value, and the default value is `RedirectConfiguration()`.
Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
Co-authored-by: Artem Redkin <artem@redkin.me>
* Added additional tests for socketPath-based requests
Motivation:
While going through the existing tests, I identified a few more instances where we could add some testing.
Modifications:
Added one test that verifies Requests are being decoded correctly, and improved three others to check for path parsing, error throwing, and schema casing respectively.
Result:
Tests that continue to pass, but that will also catch any incompatible changes in the future.
* Added some convenience initializers to URL and methods to Request for making requests to socket paths
Motivation:
Creating URLs for connecting to servers bound to socket paths currently requires some additional code to get exactly right. It would be nice to have convenience methods on both URL and Request to assist here.
Modifications:
- Refactored the get/post/patch/put/delete methods so they all call into a one line execute() method.
- Added variations on the above methods so they can be called with socket paths (both over HTTP and HTTPS).
- Added public convenience initializers to URL to support the above, and so socket path URLs can be easily created in other situations.
- Added unit tests for creating socket path URLs, and testing the new suite of convenience execute methods (that, er, test `HTTPMETHOD`s). (patch, put, and delete are now also tested as a result of these tests)
- Updated the read me with basic usage instructions.
Result:
New methods that allow for easily creating requests to socket paths, and passing tests to go with them.
* Removed some of the new public methods added for creating a socket-path based request
Motivation:
I previously added too much new public API that will most likely not be necessary, and can be better accessed using a generic execute method.
Modifications:
Removed the get/post/patch/put/delete methods that were specific to socket paths.
Result:
Less new public API.
* Renamed execute(url:) methods such that the HTTP method is the first argument in the parameter list
Motivation:
If these are intended to be general methods for building simple requests, then it makes sense to have the method be the first parameter in the list.
Modifications:
Moved the `method: HTTPMethod` parameter to the front of the list for all `execute([...] url: [...])` methods, and made it default to .GET. I also changed the url parameter to be `urlPath` for the two socketPath based execute methods.
Result:
A cleaner public interface for users of the API.
* Fixed some minor issues introduces with logging
Motivation:
Some of the convenience request methods weren't properly adapted for logging.
Modifications:
- Removed a doc comment from patch() that incorrectly referenced a logger.
- Fixed an issue where patch() would call into post().
- Added a doc comment to delete() that references the logger.
- Tests for the above come in the next commit...
Result:
Correct documentation and functionality for the patch() and delete() methods.
* Updated logging tests to also check the new execute methods
Motivation:
The logging tests previously didn't check for socket path-based requests.
Modifications:
Updated the `testAllMethodsLog()` and `testAllMethodsLog()` tests to include checks for each of the new `execute()` methods.
Result:
Two more tests that pass.
Motivation:
Due to using new ByteBuffer initializers from https://github.com/apple/swift-nio/releases/tag/2.18.0 in tests (specifically, `ByteBuffer(string: ...)`), the project will no longer compile if an older version of swift-nio is checked out.
Modifications:
Updated swift-nio from 2.16.0 to 2.18.0.
Result:
SPM should now grab the latest version of swift-nio if this version of async-http-client is used.
Motivation:
AsyncHTTPClient is not a simple piece of software and nowadays also
quite stateful. To debug issues, the user may want logging.
Modification:
Support passing a logger to the request methods.
Result:
Debugging simplified.
* Added tests for http+unix and https+unix url schemes
Motivation:
Using a base URL as the socket path only works when the URL object is maintained as long as possible through the stack. Additionally, it doesn't currently provide a way to use TLS over UNIX sockets.
Modifications:
Added two tests to test out the to-be supported URL schemes, http+unix, and https+unix, which encode the socket path as a %-escaped hostname, as some existing services already do.
Result:
Better UNIX domain socket support.
Motivation:
Previously, we'd only use the server's connection header to determine if
we should close the connection or not. That's wrong because if we set
`connection: close` ourselves, we must not reuse again.
Modification:
Set `TaskHandler.closing = false` if we send a close header.
Result:
More HTTP correctness.
Motivation:
We didn't have a test that tested that streaming really streams, ie.
that request body part N arrives before N+1 is sent.
Modification:
Test it.
Result:
Better test coverage.
Motivation:
AHC's tests were split in HTTPClientTests (which only use the public
API) and HTTPClientInternalTests (which can use `internal` API by using
`@testable`). At some point, the public API tests had `@testable` added
to the AHC import which breaks this idea.
Modification:
Restore the intent by removing `@testable` and moving the 2 tests that
needed it over.
Result:
Cleaner test suite.
This is the continuation of the good work of @Yasumoto and @weissi in #135
The following code adds support for NIO Transport services. When the ConnectionPool asks for a connection bootstrap it is returned a NIOClientTCPBootstrap which wraps either a NIOTSConnectionBootstrap or a ClientBootstrap depending on whether the EventLoop we are running on is NIOTSEventLoop.
If you initialize an HTTPClient with eventLoopGroupProvider set to .createNew then if you are running on iOS, macOS 10.14 or later it will provide a NIOTSEventLoopGroup instead of a EventLoopGroup.
Currently a number of tests are failing. 4 of these are related to the NIOSSLUncleanShutdown error the others all seem related to various race conditions which are being dealt with on other PRs. I have tested this code with aws-sdk-swift and it is working on both macOS and iOS.
Things look into:
The aws-sdk-swift NIOTS HTTP client had issues with on Mojave. We should check if this is the case for async-http-client as well.
Co-authored-by: Joe Smith <yasumoto7@gmail.com>
Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
* Enable clients to call URLs that include %2F as an escaped backslash
Previously `percentEncodedPath` was using `path.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed)`
which converts %2F to a literal '/'. This prevented users fetching URLs like https://api.travis-ci.org/repo/github/rails%2Frails
which use %2F as part of a path segment.
Migrating to `URLComponents(url: self, resolvingAgainstBaseURL: false)?.percentEncodedPath` has the desired behaviour
for the couple of test cases that exist.
Updated the test server to switch on the `percentEncodedPath` so it's easier
to understand the desired behaviour.
Artem Redkin
Artem Redkin
Tim Condon
Adam Fowler
Cory Benfield
Max Desiatov
Dimitri Bouniol
Johannes Weiss
Fabian Fett
Iain Smith
tomer doron