* 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:
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.
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.
motivation:
TLS handshake promise was leaked in some cases of failure (see #179)
changes:
- Avoid leaking promise
- Clearer completion flow for related futures
- Add testAvoidLeakingTLSHandshakeCompletionPromise test
* Close idle pool connections
Motivation: Pooled connections should close at some point (see #168)
Changes:
- Add new poolingTimeout property to HTTPClient.Configuration, it's
default value is .seconds(60), it can be set to nil if one wishes to
disable this timeout.
- Add relevant unit test
Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
* Remove activity assertion in prepareForClose
motivation: assertion on activity sometimes fails but it turns out it is overly restrictive
changes:
- remove assertion and accept any given activity in prepareForClose
* test closing connections whilst syncShutdown
Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
motivation: Better performance thanks to connection reuse
changes:
- Added a connection pool for HTTP/1.1
- All requests automatically use the connection pool
- Up to 8 parallel connections per (scheme, host, port)
- Multiple additional unit tests
Previously, UNIX Domain Sockets would only work if the URL also had a
"base URL". If it didn't have a base URL, it would try to connect to the
empty string which would fail :).
Now, we support both cases:
- URLs with a baseURL (the path to the UDS) and a path (actual path)
- URLs that just have an actual path (path to the UDS) where we'll just
use "/" as the URL's path
As discussed in #128. We make the ResponseAccumulator public to give developers an easy time to create a Task. With the ResponseAccumulator the developer using this does not have to worry, about implementing the HTTPClientResponseDelegate all by himself.
* Bugfix HTTPS SNI and IP Address
Motivation:
Solving the SNI Bug
Modifications:
Added an internal extension on String for checking if the hostname is an IP Address -- see the private extension on SNI. Additionally using the IPv4Address and IPv6Address Function from Network above 10.14 as protecting with #availabe.
Adding the test for HTTPS and IP in as hostname
Result:
We get results with an IP as Hostname
Motivation:
More unit tests are good and now that SwiftNIO shipd
`NIOHTTP1TestServer`, writing integration tests for async-http-client is also
more straightforward.
Modification:
Demonstrate some tests using NIOHTTP1TestServer.
Result:
More tests.
Artem Redkin
Dimitri Bouniol
Max Desiatov
Artem Redkin
Johannes Weiss
Adam Fowler
Fabian Fett
Iain Smith
Trevör
Marcin Krzyzanowski
Andreas Kostuch