Validate configuration at initialization by aryan-25 · Pull Request #107 · swift-server/swift-http-server

aryan-25 · 2026-08-03T16:01:44Z

Motivation

The required TLS contexts/configurations required to create the server channels are only constructed when NIOHTTPServer.serve is invoked. The problem with this is that these constructions can throw an error if the provided configuration was invalid.

We should not allow an invalid NIOHTTPServerConfiguration instance to be created in the first place: validation should occur during the initialization of NIOHTTPServerConfiguration.

Modifications

NIOHTTPServerConfiguration's initializer now validates that supportedHTTPVersions and transportSecurity are compatible with each other, and stores the necessary TLS objects.
- NIOHTTPServer.serve .serve can therefore use these properties directly and not invoke any throwing method relating to configuration.
Some small general refactors to improve readability.

Result

Validation of NIOHTTPServerConfiguration now happens at initialization rather than during NIOHTTPServer.serve.

gjcairo · 2026-08-04T12:59:06Z

+        @available(anyAppleOS 26.0, *)
+        @Test(
+            "All X.509 credential sources produce a valid configuration",
+            arguments: [TestX509CredentialSource]([.inMemory, .reloading, .pemFile, .derFile, .pemBytes, .derBytes])


Nit, but for consistency with other tests/files/packages:

Suggested change

arguments: [TestX509CredentialSource]([.inMemory, .reloading, .pemFile, .derFile, .pemBytes, .derBytes])

arguments: [TestX509CredentialSource.inMemory, .reloading, .pemFile, .derFile, .pemBytes, .derBytes]

This has to change in a few places.

Addressed in commits 8da01f8 and bd7757d.

gjcairo · 2026-08-04T13:04:25Z

+        func nonExistentX509FilePathRejected(
+            credentials: NIOHTTPServerConfiguration.TransportSecurity.X509Credentials
+        ) throws {
+            #expect(throws: Error.self) {


We can't assert a more specific error type or message?

Different errors are returned in both cases by swift-nio-ssl. The PEM case throws a NIOSSLError.failedToLoadCertificate error and the DER case throws a NIOCore.IOError.

I felt that explicitly catching these errors clutters the test case without adding much benefit.

I only worry that expecting any error may mean that even though this behaviour changes in the future (e.g. a new error we should be handling different starts being thrown), the test will still pass.

gjcairo · 2026-08-04T13:23:23Z

+}
+
+@available(anyAppleOS 26.0, *)
+enum TestX509CredentialSource: Sendable {


We don't have a type like this in the actual configuration types? Feels a bit weird to have this parallel type in the tests instead of an extension on that type similarly to what we're doing below with NIOHTTPServerConfiguration.TransportSecurity.RawPublicKeyCredentials.

The actual configuration types don't allow us to express the cases (e.g. inMemory, reloading, etc.) without providing associated values.

These parallel types allow us to parameterise over all cases quite easily. There is only one supported format for RPK credentials (DER files), which is why I didn't bother for that case.

gjcairo · 2026-08-04T13:23:37Z

+#endif  // HTTP3
+
+@available(anyAppleOS 26.0, *)
+enum MTLSTrustSource: Sendable {


Same question here

Answered here.

gjcairo · 2026-08-04T13:25:13Z

+extension NIOHTTPServerConfiguration.HTTPVersion {
+    static let http2 = Self.http2(config: .defaults)
+
+    #if HTTP3
+    static let http3 = Self.http3(config: .defaults)
+    #endif
+}


Perhaps we should default config to .defaults in the init instead of having this

Addressed in commit 6605bc2.

gjcairo · 2026-08-04T13:37:36Z


    /// Network binding configuration specifying all addresses where the server should listen.
-    public var bindTargets: [BindTarget]
+    public let bindTargets: [BindTarget]


We generally keep properties in config structs as vars so they can be modified. This is common across all of our packages AFAIK. Did you make this change for a specific reason?

Yes. If any of these three properties (bindTargets, transportSecurity, supportedHTTPVersions) are modified after initialization, then the two new contexts we store may become invalid.

We can add a willSet to these properties and do the validation/update of the contexts there.

We discussed offline and because we can't have throwing setters we will probably just precondition failure.
Thinking more about it though, because this wouldn't fail on release builds, I think it's better to avoid having the ValidatedX types and instead just do validations in the init/setters, without encapsulating in separate types.

I've now updated the PR with this approach.

gjcairo · 2026-08-05T16:19:53Z

        ///
        /// - Parameter config: The configuration to use for HTTP/2.
-        public static func http2(config: HTTP2) -> Self {
+        public static func http2(config: HTTP2 = .defaults) -> Self {


Nit, but I think additionally having a static var for h2 and h3 that defaults the config to .defaults may make the API slightly nicer since we can avoid the empty brackets (.http2 instead of .http2())

Addressed in commit b32144d.

FranzBusch · 2026-08-09T15:37:56Z

This is interesting. A potential side observation: This seems to boil down to differences in support transport security for H1/2/3. It would be great if this is a compile time guarantee rather than a runtime enforced. Have we considered making the transport security structs HTTP version specific?

aryan-25 · 2026-08-11T08:46:52Z

This seems to boil down to differences in support transport security for H1/2/3

Yes. Although it is worth nothing that most of the differences today just arise from a mismatch in the credential sources supported by swift-nio-ssl and swift-nio-quic. From swift-nio-quic, we need support for (1) byte-array, DER-file, and in-memory swift-certificates sources, (2) certificate reloading, and (3) mTLS (with support for a custom verification callback). From swift-nio-ssl, we need support for Raw Public Key sources.

Once there is parity, the only difference that remains is transportSecurity == .plaintext being possible over HTTP/1.1 (and HTTP/2 once we add support for h2c), but not over HTTP/3.

It feels odd to have a transportSecurity type for HTTP/3 and a very similar one for HTTP/1.1 / HTTP/2 that additionally just allows plaintext to be expressed. Having a transportSecurity per supported HTTP version may not also be the right shape because when supportedHTTPVersions == [.http1_1, .http2], peers may be intending to use either HTTP/1.1 or HTTP/2, but we only find out through the TLS handshake (ALPN), which means we can only support a single set of TLS credentials.

gjcairo · 2026-08-11T09:18:10Z

+    ///   - `transportSecurity` can only be set to `.plaintext` when `supportedHTTPVersions == [.http1_1]`.
+    public var transportSecurity: TransportSecurity {
+        didSet {
+            try! self.validateTransportConfiguration()


I don't think we should try! here. I think it's okay to precondition failure because we won't crash at runtime on release builds, but with this we could. We should throw in throwing contexts and precondition failure where we can't throw.

Note that preconditionFailure will also crash in release builds (assuming standard release build configs). It will only not crash when the package is built with -Ounchecked, which is a non-standard release build config: https://developer.apple.com/documentation/swift/preconditionfailure(_:file:line:)#discussion.

But I've removed the try! and replaced it with preconditionFailure for consistency in commit 5c29ae1.

FranzBusch · 2026-08-12T10:45:06Z

Yes. Although it is worth nothing that most of the differences today just arise from a mismatch in the credential sources supported by swift-nio-ssl and swift-nio-quic. From swift-nio-quic, we need support for (1) byte-array, DER-file, and in-memory swift-certificates sources, (2) certificate reloading, and (3) mTLS (with support for a custom verification callback). From swift-nio-ssl, we need support for Raw Public Key sources.

Once there is parity, the only difference that remains is transportSecurity == .plaintext being possible over HTTP/1.1 (and HTTP/2 once we add support for h2c), but not over HTTP/3.

It feels odd to have a transportSecurity type for HTTP/3 and a very similar one for HTTP/1.1 / HTTP/2 that additionally just allows plaintext to be expressed. Having a transportSecurity per supported HTTP version may not also be the right shape because when supportedHTTPVersions == [.http1_1, .http2], peers may be intending to use either HTTP/1.1 or HTTP/2, but we only find out through the TLS handshake (ALPN), which means we can only support a single set of TLS credentials.

I understand where the current differences come from. Just to explain this a bit more. What if a user wants to bootstrap a server that serves H/1 over plaintext and H2 over TLS? This is currently not configurable right? This might be fine just trying to provide another argument why separate H1/2/3/ properties might be beneficial. I agree though that they should use the same type.

aryan-25 added 3 commits August 3, 2026 16:49

Validate configuration at initialization

043c39d

Refactor

f933d14

Refactor

f6b994d

aryan-25 requested a review from gjcairo August 3, 2026 16:01

aryan-25 added the ⚠️ semver/major Breaks existing public API. label Aug 3, 2026

gjcairo requested changes Aug 4, 2026

View reviewed changes

aryan-25 added 6 commits August 5, 2026 13:47

Merge branch 'main' into validate-config-at-init

9aa1c85

Format argument declaration

8da01f8

Set default value for HTTP/2 and HTTP/3 configuration

6605bc2

Use CaseIterable

bd7757d

Add Validated prefix to context types

f20d89f

Merge branch 'main' into validate-config-at-init

3d44436

gjcairo reviewed Aug 5, 2026

View reviewed changes

aryan-25 added 4 commits August 6, 2026 17:17

Add static var for default http2 and http3 configurations

b32144d

Remove validated types; store validated contexts inline

54b6357

Fix DocC references

539385f

Refactor

a1658a9

aryan-25 requested a review from gjcairo August 6, 2026 16:53

aryan-25 added 2 commits August 6, 2026 18:10

Reduce duplication in tests

d970498

Empty commit to re-trigger CI checks

0ece720

Empty commit to re-trigger CI checks

a8abebb

gjcairo reviewed Aug 11, 2026

View reviewed changes

Replace try! with preconditionFailure

5c29ae1

aryan-25 requested a review from gjcairo August 12, 2026 08:31

gjcairo approved these changes Aug 12, 2026

View reviewed changes

	arguments: [TestX509CredentialSource]([.inMemory, .reloading, .pemFile, .derFile, .pemBytes, .derBytes])
	arguments: [TestX509CredentialSource.inMemory, .reloading, .pemFile, .derFile, .pemBytes, .derBytes]

Uh oh!

Conversation

aryan-25 commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Result

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FranzBusch commented Aug 9, 2026

Uh oh!

aryan-25 commented Aug 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FranzBusch commented Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aryan-25 commented Aug 3, 2026 •

edited

Loading