`NIOTypedHTTPClientUpgradeHandler.handlerAdded` will write upgrade
request if the channel is already active
### Motivation:
This has been added to make it easier to implement a websocket client
that supports proxies. If the upgrade handler is added after the proxy
connect has been processed previously nothing would happen.
### Modifications:
- Added new function
`NIOTypedHTTPClientUpgradeHandler.writeUpgradeRequest()` function. Call
this from `channelActive` and also from `handlerAdded` if the channel is
already active.
- Added test `testUpgradeHappensAfterHandlerAdded`
### Result:
NIOTypedHTTPClientUpgradeHandler now also works if it is added to a
channel after the channel is active
---------
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
NIOHTTPResponseHeadersValidator drops head/end response parts if they
contain invalid header fields. If a head part is dropped and the server
allows pipelining then the subsequent body/end part will reach the
pipelining handler and result in an assertion failure in debug builds.
In release builds, some parts may be incorrectly written out leading to
protocol violations.
Modifications:
- Drop all response parts after an invalid response part
Result:
- Fewer bugs
- Resolves#3326
Co-authored-by: Cory Benfield <lukasa@apple.com>
Fixes all warnings when `-require-explicit-sendable` flag is enabled and
enables the flag on macOS CI.
### Motivation:
We want to ensure our public API is either explicitly marked as
`Sendable` or not.
### Modifications:
Marked appropriate public types as `Sendable`, or explicitly defined
their conformance to the `Sendable` protocol as unavailable.
### Result:
We can now enable `-require-explicit-sendable` compiler flag in our
codebase.
Motivation:
Swift 5.9 is no longer supported, we should bump the tools version and
remove it from our CI.
Modifications:
* Bump the Swift tools version to Swift 5.10
* Remove Swift 5.9 jobs where appropriate in main.yml, pull_request.yml
Result:
Code reflects our support window.
Motivation:
Public static lets can serve a bunch of roles, but one of them is to
store simple constants: integers, and other trivial types, for example.
This is a nice pattern and for internal and private static lets it works
well, but for public ones it produces some inefficient code.
In particular, it has two downsides. First, it allocates storage for
that value. We don't actually need to allocate a few hundred extra
megabytes for the various integers we want to store.
Secondly, it forces calling code to access the address and call the
dispatch_once code in order to get hold of the value. For trivial types
we don't need that cost: they can just know what the value is directly.
Inlinable computed vars avoid all of these costs: they have no size
overhead for storage, and they are visible to all clients so their
values can be directly assembled.
While I'm here, I added a bunch of other inlinable annotations for a few
trivial data types I stumbled onto.
Modifications:
- Added loads of inlinables. Like, loads.
- Swapped many static lets to static vars.
Result:
Better codegen, smaller memory footprints, more attributes.
To handle errors fired by previous ChannelHandlers. eg
NIOSSLClientHandler
### Motivation:
Otherwise if a ChannelHandler previous to the upgrade ChannelHandler
fails the error returned is `inappropriateOperationForState` and the
error fired by the original ChannelHandler is lost.
Added `testReturnsErrorsFromPriorChannelHandlers`. Also had to
reorganise `setUpClientChannel` slightly so I could get the result of
the upgrade.
---------
Co-authored-by: Cory Benfield <lukasa@apple.com>
### Motivation:
To ensure NIOHTTP1 concurrency safety.
### Modifications:
* Enable strict concurrency checking in the package manifest.
* Mark several objects `Sendable` with `@preconcurrency` annotations
where they are returned in futures which may execute in arbitrary
concurrency domains.
* `NIOTypedHTTPClientProtocolUpgrader`
* `NIOTypedHTTPClientUpgradeConfiguration`
* `NIOUpgradableHTTPServerPipelineConfiguration`
* `NIOUpgradableHTTPClientPipelineConfiguration`
* `NIOTypedHTTPServerProtocolUpgrader`
* `NIOTypedHTTPServerUpgradeConfiguration`
* Mark handlers as explicitly not sendable
* `NIOTypedHTTPClientUpgradeHandler`
* `NIOTypedHTTPServerUpgradeHandler`
* Added new Sendable type aliases:
* `NIOHTTPClientUpgradeSendableConfiguration`
* `NIOHTTPServerUpgradeSendableConfiguration`
### Result:
No more concurrency warnings. Builds will warn and CI will fail if
regressions are introduced.
Adds a new `ChannelPipeline.SynchronousOperations.Position` type to fill
a hole in the API that prevented non-sendable Channel handlers being
added at a position to the pipeline via the synchronous operations.
---------
Co-authored-by: Cory Benfield <lukasa@apple.com>
### Motivation:
As outlined in this
[issue](https://github.com/apple/swift-nio/issues/2930) by @weissi, the
current performance of calling `description` on `HTTPHeaders` is
undermined by the dynamism of Array.
### Modifications:
The proposed solution replaces the implementation of `description` to
iterate over the items and print them out manually. This provides a
faster solution since we bypass the cost of calling into `description`
of an Array.
### Result:
A more performant implementation of `HTTPHeaders` description.
---------
Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
Swift 6.1 nightlies have added many new warnings and errors to NIO. This
patch resolves all of them.
Modifications:
- Clean up missing imports
- Move away from @_implementationOnly where necessary
- Fix a few things where Sendable mismatches were present.
Result:
Clean compile again
---------
Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
### Motivation:
Opening the `swift-nio` repository made me warning blind because there
were always so many trivially fixable warnings about things that were
correct but cannot be understood by the compiler.
### Modifications:
Fix all the sendable warnings that popped up, except for one test where
`NIOLockedValueBox<Thread?>` isn't sendable because `Foundation.Thread`
seemingly isn't `Sendable` which is odd. Guessing that'll be fixed on
their end.
### Result:
- Fewer warnings
- Less warning-blindness
- More checks
Check for half closure during server upgrade and close channel if client
closes the channel
### Motivation:
This is to fix#2742
### Modifications:
Add `userInboundEventTriggered` function to
`NIOTypedHTTPServerProtocolUpgrader` which checks for
`ChannelEvent.inputClosed`
### Result:
Negotiation future now errors when client closes the connection instead
of never completing
---------
Co-authored-by: Franz Busch <privat@franz-busch.de>
### Motivation:
Documentation checking catches more issues in Swift 6.0.
### Modifications:
Adopt the Swift 6.0 image and fix the errors.
### Result:
More accurate docs.
Motivation:
RemovableChannelHandlers have a large API surface in NIOCore. That API
surface is a bit awkward with regard to strict concurrency, and needs
some cleanup.
Modifications:
This patch adds some new API that is necessary to safely work with
RemovableChannelHandlers, deprecates some API that cannot plausibly be
used, and cleans up some other parts of the API.
Result:
Easier to work with RemovableChannelHandlers
* Apply formatting
* Apply no block comments rule
* Apply OmitExplicitReturns
* Apple OnlyOneTrailingClosureArgument
* Apply NoAssignmentInExpressions
* Fix up DontRepeatTypeInStaticProperties lint errors
* Apply `OrderedImports`
* Apply `ReplaceForEachWithForLoop`
* format file
* Enable the formatting pipeline
* Adopt `AmbiguousTrailingClosureOverload`
* Fix license header
* Fix format check
* Fix `EndOfLineComment`
* Fix CI
* Adapt CI script to check if changes when running formatting
* Separate lint and format into to steps
* Fix format
* Adopt `UseEarlyExits`
* Revert "Adopt `UseEarlyExits`"
This reverts commit d1ac5bbe12.
* Fixed an issue where HTTP/2 connections would disconnect if response compression were used and a 304 Not Modified response was returned
* Added HTTP encoder tests for 304 Not Modified responses
# Motivation
Adding a handler via the regular `pipeline.addHandler` APIs can lead to transferring the handler over isolation regions. To avoid running into `Sendable` warnings here we can use the `syncOperations` of the pipeline to avoid transferring the handlers.
# Modification
This PR is mostly changing code to avoid transferring handlers to and from the event loops in tests. This should be a no-op mostly.
Co-authored-by: Cory Benfield <lukasa@apple.com>
* Revert "Back out new typed HTTP protocol upgrader (#2579)"
# Motivation
We have reverted the typed HTTP protocol upgrader pieces since adopters were running into a compiler bug (https://github.com/apple/swift/pull/69459) that caused the compiler to emit strong references to `swift_getExtendedExistentialTypeMetadata`. The problem is that `swift_getExtendedExistentialTypeMetadata` is not available on older runtimes before constrained existentials have been introduced. This caused adopters to run into runtime crashes when loading any library compiled with this NIO code.
# Modifications
This PR reverts the revert and guard all new code in a compiler guard that checks that we are either on non-Darwin platforms or on a new enough Swift compiler that contains the fix.
# Result
We can offer the typed HTTP upgrade code to our adopters again.
* Add compiler guards
# Motivation
We got reports in https://github.com/apple/swift-nio/issues/2574 that our new typed HTTP upgrader are hitting a Swift compiler bug which manifests in a runtime crash on older iOS/macOS/etc.
# Modification
This PR backs out the new typed HTTP protocol upgrader APIs so that we can unblock our users until the Swift compiler bug is fixed.
# Result
No more crashes for our users.
* Avoid terminating when a precondition is not met in HTTPServerPipelineHandler
* Address review comments
* rename precondition to assertion
---------
Co-authored-by: Cory Benfield <lukasa@apple.com>
# Motivation
Over the past months, we have been working on new async bridges to make using NIO's `Channel` from Swift Concurrency possible. Since this work was far reaching we have opted to land all of it as SPI. Now the time has come and we feel confident enough to make the SPI official API. This comes after testing the new APIs in various scenarios such as HTTP 1&2, HTTP upgrades, protocol negotiation and in benchmarks.
# Modification
This PR removes the SPI from the `NIOAsyncChannel`, the bootstrap methods, protocol negotiation and HTTP upgrade.
# Result
Everyone can use the our new APIs🚀
* Introduce new typed `HTTPClientUpgrader` and `WebSocketClientUpgrader`
# Motivation
In my previous PR https://github.com/apple/swift-nio/pull/2517, I added a new typed `HTTPServerUpgrader` and corresponding implementation for the `WebSocketServerUpgrader`. The goal of those is to carry type information across HTTP upgrades which allows us to build fully typed pipelines.
# Modification
This PR adds a few things:
1. A new `NIOTypedHttpClientUpgradeHandler` + `NIOTypedHttpClientProtocolUpgrader`. I also moved the state handling to a separate state machine. Similar to the server PR I did not unify the state machine between the newly typed and untyped upgrade handlers since they differ in logic.
2. A new `NIOTypedWebSocketClientUpgrader`
3. An overhauled WebSocket client example.
# Result
This is the last missing piece of dynamic pipeline changing where we did not carry around the type information. After this PR lands, we can finalize the `AsyncChannel` and async typed NIO pieces.
* Remove availability on the protocols
* Tolerate empty HTTP response body parts
### Motivation
Empty HTTP response body parts currently eagerly end a response. This is unexpected and against NIOs documented behaviour.
### Modification
- add test which send an empty response body part and would previously fail
- skip empty response body parts
### Result
Users can send empty body parts without ending the response.
* add comment explaining why we do it here instead of fixing it at the source
* move empty body part logic to `writeChunk`
* Introduce new typed `HTTPServerUpgrader` and `WebSocketServerUpgrader`
# Motivation
With our new `NIOAsyncChannel` and typed bootstrap APIs we want to be able to let users spell out their pipeline in a typed way. Pipelines can contain handlers that have to make a forking decision such as HTTP upgrading. Our current `HTTPServerUpgradeHandler` is one of those handlers but it lacks strict typing. To interact nicely with our new typed APIs we need to have a new variant of the `HTTPServerUpgradeHandler` that can carry type information.
# Modification
This PR adds a few things:
1. A new `NIOTypedHTTPServerUpgradeHandler` + `NIOTypedHTTPServeProtocolUpgrader`. I also moved the state handling logic to a separate state machine. I thought about unifying the state machines of the _old_ handler and the new one but they differ in behaviour which makes the state machine more complicated.
2. A new `NIOTypedWebSocketServerUpgrader` that conforms to `NIOTypedHTTPServerProtocolUpgrader`
3. An overhauled WebSocket server example that fully uses Concurrency.
# Result
We now have a way to fully type the server side of HTTP protocol upgrading.
Code review
Update parameter names for new API and fix example
* Introduce new configuration struct and rename to `UpgradablePipeline`
* Review comments
* Bump minimum Swift version to 5.7
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 integration test script
* Update docs
Result:
Remove support for Swift 5.6, add 5.10
* fix indentation issues
* 5.9 docker image use release image
Motivation:
Right now we always set the framing headers in HTTP requests and
responses that we send. This is a good practice for most users, but
it can cause issues, most notably in responses to CONNECT requests
which requires that we do not set framing headers.
Unfortunately, in NIO's current HTTP/1.1 design it is not possible
for us to suppress these headers. This is because the HTTP encoders
come _earlier_ in the pipeline than the decoders, so the HTTP
encoders do not know structurally what requests they are responding
to.
While we could merge the encoders/decoders, that's a fairly
substantial change. A better short-term change is to offer users
the ability to turn off the NIO header manipulation feature. In
this circumstance, users take on full responsibility for
appropriately framing their HTTP messages.
Modifications
- Add config to HTTPRequestEncoder and HTTPResponseEncoder.
- Plumb this config through.
- Add a bunch of tests.
Result
Users have way more control of HTTP/1.1 messages.
Motivation
We shouldn't crash on somewhat likely user error.
Modifications
Pass on writes after close(mode: .output) instead of crashing.
Result
User code is more robust to weird edge cases.
Motivation:
Currently the server pipeline handler ignores close. This generally works,
except in the rare case that the user calls close(mode: .output). In this
instance they have signalled that they'll never write again, and they're
likely expecting a final close shortly after.
However, it is possible that the pipeline handler has suspended reads
at the same time. On Linux this isn't an issue, because we'll still be told
about the eventual socket close. However, on Apple platforms we won't: we've
masked off the reads, and we can't listen to EVFILT_EXCEPT due to some
other issues. This means that on Apple platforms the server pipeline handler
can accidentally wedge the Channel open and prevent it from closing.
We should take this opportunity to have the server pipeline handler be smart
about close(mode: .output). What _should_ happen here is that the pipeline
handler should immediately refuse to deliver further requests on the Channel.
If one is in-flight, it can continue, but everything else should be dropped.
This is because the server cannot possibly respond to further requests.
Modifications:
- Add new states to the server pipeline handler
- Drop buffered requests and new data after close(mode: .output)
- Add tests
Result:
Server pipeline handler behaves way better.
Motivation:
The `HTTPServerUpgradeHandler` removes itself from the pipeline after an
upgrade and unbuffers any unconsumed reads. If an upgrade starts but
does not complete successfully then the pipeline may be left in an
unknown state.
If, for example, the failure occurs before the user provided upgrade
handler is run then unbuffered writes may be unwrapped by the wrong
channel handler as the wrong type leading to a crash.
Modifications:
- Only remove the upgrade handler and forward buffered writes if all
parts of the upgrade complete successfully.
- If part of the upgrade fails then fire an error into the channel
pipeline without removing the server upgrade handler.
- Remove a few unnecessary `map`s.
- Make `httpEncoder` non-optional since and remove associated dead code
since it can never be `nil`.
Result:
Upgrade handling is safer.
Motivation
When we receive a HEAD response, it's possible that the response
contains a content-length. llhttp has a bug
(https://github.com/nodejs/llhttp/issues/202) that prevents it from
properly managing that issue, which causes us to incorrectly parse
responses.
Modifications
Forcibly set llhttp's content-length value to 0.
Result
Correctly handle HTTP framing around llhttp's issues.
Motivation
HTTP headers are prevented from containing certain characters that can
potentially affect parsing or interpretation. Inadequately policing this
can lead to vulnerabilities in web applications, most notably HTTP
Response Splitting.
NIO was insufficiently policing the correctness of the header fields we
emit in HTTP/1.1. We've therefore added a new handler that is
automatically added to channel pipelines that will police the validity
of header fields.
For projects that are already running the validation themselves, this
can be easily disabled. Note that by default NIO does not validate
content length is correctly calculated, so applications can have their
framing fall out of sync unless they appropriately calculate this
themselves or use chunked transfer encoding.
Modifications
- Add thorough unit testing to confirm we will not emit invalid header
fields.
- Error if a user attempts to send an invalid header field.
Result
NIO applications are no longer vulnerable to response splitting by CRLF
injection by default.
Motivation:
We should better tolerate LLHTTP status codes we don't yet know about.
Modifications:
- Added support for the status codes that currently exist
- Add a fallback to the RAW case for the future.
Result:
Better management of LLHTTP status codes
Motivation:
The node.js HTTP parser library that we use has been unmaintained for some time. We should move to the maintained replacement, which is llhttp. This patch will update our dependency and bring us over to the new library, as well as make any changes we need.
Modifications:
This patch comes in 4 parts, each contained in a separate commit in the PR.
The first commit drops the existing http_parser code and updates some of the repo state for using llhttp.
The second commit rewrites the update script to bring in llhttp instead of http_parser.
The third runs the actual script. You can skip reviewing this except to sanity check the outcome.
The fourth commit updates the NIO code and the tests to get everything working.
In general the substance of the product modifications was minimal. The logic around keeping track of where we are in the buffer and how upgrades work has changed a bit, so that required some fiddling. I also had to add an error reporting path for the delegates to be able to throw specific errors that llhttp no longer checks for. Finally, I removed two tests that were a little overzealous and that llhttp does not police.
Result:
Back on the supported path.
* Adopt `Sendable` for closures in `HTTPPipelineSetup.swift`
* make `UnsafeTransfer` and `UnsafeMutableTransferBox` available in Swift 5.4 too
* fix code duplication
* Copy `UnsafeTransfer` to `NIOHTTP1Tests` to be able to remove `@testable` from `NIOCore` import
* Adopt `Sendable` for closures in `HTTPServerUpgradeHandler.swift`
* Adopt `Sendable` for closures in `HTTPClientUpgradeHandler.swift`
* Adopt `Sendable` for closures in `HTTPPipelineSetup.swift`
* make `UnsafeTransfer` and `UnsafeMutableTransferBox` available in Swift 5.4 too
* fix code duplication
* Copy `UnsafeTransfer` to `NIOHTTP1Tests` to be able to remove `@testable` from `NIOCore` import
# Motivation
`HTTPResponseStatus` is currently not conforming to `Hashable`.
# Modification
Add `Hashable` conformance to `HTTPResponseStatus`
# Result
`HTTPResponseStatus` is `Hashable`
Adam Fowler
aryan-25
George Barnett
Fabian Fett
Rafael Cepeda
Rick Newton-Rogers
Cory Benfield
Tim Condon
Mirza Učanbarlić
Johannes Weiss
Franz Busch
Dimitri Bouniol
hamzahrmalik
David Nadoba
Rick Newton-Rogers
Alex Taffe
carolinacass