From f6a77305ac4c881f66c459a5ff1dca9b8f018fa7 Mon Sep 17 00:00:00 2001 From: Javier Cicchelli Date: Sun, 23 Aug 2026 00:06:13 +0000 Subject: [PATCH] Trailing slash middleware for the Infrastructure package (#53) --- Packages/Infrastructure/README.md | 4 +- .../TrailingSlashRedirectMiddleware.swift | 114 +++++++++++++ ...TrailingSlashRedirectMiddlewareTests.swift | 150 ++++++++++++++++++ Services/Website/README.md | 20 ++- .../Sources/App/Extensions/App+Build.swift | 7 +- Services/Website/docker-compose.override.yml | 1 + Services/Website/docker-compose.yml | 7 +- 7 files changed, 293 insertions(+), 10 deletions(-) create mode 100644 Packages/Infrastructure/Sources/Public/Middlewares/TrailingSlashRedirectMiddleware.swift create mode 100644 Packages/Infrastructure/Tests/Cases/Public/Middlewares/TrailingSlashRedirectMiddlewareTests.swift diff --git a/Packages/Infrastructure/README.md b/Packages/Infrastructure/README.md index f6545b8..0cd6239 100644 --- a/Packages/Infrastructure/README.md +++ b/Packages/Infrastructure/README.md @@ -5,7 +5,7 @@ The shared [Hummingbird](https://github.com/hummingbird-project/hummingbird) too | Role | Types | | --- | --- | | Routing | `RouterController`, `RouteCollectionBuilder`, the `addController` extension on `RouterMethods` | -| Middlewares | `SecurityHeadersMiddleware`, `HTTPSRedirectMiddleware`, `VaryMiddleware`, `RateLimitMiddleware`, `LocalizationMiddleware`, `NotFoundMiddleware` | +| Middlewares | `SecurityHeadersMiddleware`, `HTTPSRedirectMiddleware`, `TrailingSlashRedirectMiddleware`, `VaryMiddleware`, `RateLimitMiddleware`, `LocalizationMiddleware`, `NotFoundMiddleware` | | Pages and assets | `Page`, `Asset`, `AssetExtension`, `FingerprintAssets` | | Link previews | `SocialCard`, its `Image`, and the `Tag` meta tags it derives | | Structured data | `StructuredData`, the `Node`, `Property`, and `Value` types of its schema.org graph, and the open `Name` and `Kind` vocabularies | @@ -30,7 +30,7 @@ Sources/ │ ├── Enumerations/ AssetExtension │ ├── Extensions/ addController, plus the default header names and values │ ├── Methods/ FingerprintAssets -│ ├── Middlewares/ the six HTTP middlewares +│ ├── Middlewares/ the seven HTTP middlewares │ ├── Protocols/ Asset, LocalizedRequestContext, Page, RouterController │ ├── Responses/ CachedHTMLResponse, LocalizedHTMLCollectionResponse │ └── Types/ Analytics, SocialCard, StructuredData (the latter two nesting their own types in SocialCard/ and StructuredData/) diff --git a/Packages/Infrastructure/Sources/Public/Middlewares/TrailingSlashRedirectMiddleware.swift b/Packages/Infrastructure/Sources/Public/Middlewares/TrailingSlashRedirectMiddleware.swift new file mode 100644 index 0000000..39344ab --- /dev/null +++ b/Packages/Infrastructure/Sources/Public/Middlewares/TrailingSlashRedirectMiddleware.swift @@ -0,0 +1,114 @@ +import HTTPTypes +import Hummingbird + +/// Answers a request whose path carries a trailing slash with `301 Moved Permanently` to the same path without one. +/// +/// The router matches `/mr-rock` and `/mr-rock/` alike, and `FileMiddleware` serves `/robots.txt/` as readily as `/robots.txt`, so without this +/// every address on the site answers under at least two URLs. A search engine treats those as separate pages competing with each other, and any link +/// earned by one is not credited to the other. Redirecting collapses them onto the form the pages already name as canonical. +/// +/// Two deliberate choices: +/// - **The `Location` is relative.** A relative target resolves against the request's own scheme and host, so the middleware needs no configured +/// origin and cannot be aimed elsewhere by a forged `Host` header. +/// - **Only `GET` and `HEAD` are redirected.** A client answering a `301` to a `POST` may repeat it as a `GET` and drop the body, so a form +/// submission is left to the route that already matches it. +/// +/// - Note: `Context` is the request context the middleware is resolved against. +public struct TrailingSlashRedirectMiddleware: Sendable { + + // MARK: Initializers + + /// Creates a trailing-slash redirect middleware. + public init() {} + +} + +// MARK: - RouterMiddleware + +extension TrailingSlashRedirectMiddleware: RouterMiddleware { + + // MARK: Functions + + /// Redirects a `GET` or `HEAD` whose path carries a trailing slash, and passes every other request down the chain. + /// - Parameters: + /// - request: the incoming request. + /// - context: the context the request is resolved against. + /// - next: the next responder in the middleware chain. + /// - Returns: the redirect, or the downstream response. + /// - Throws: any error thrown downstream. + public func handle( + _ request: Request, + context: Context, + next: (Request, Context) async throws -> Response + ) async throws -> Response { + let path = request.uri.path + let canonical = canonicalPath(of: path) + + guard + request.method == .get || request.method == .head, + canonical != path + else { + return try await next( + request, + context + ) + } + + var response = Response(status: .movedPermanently) + + response.headers[.location] = canonical + query(of: request) + + return response + } + +} + +// MARK: - Helpers + +private extension TrailingSlashRedirectMiddleware { + + // MARK: Methods + + /// The path with its trailing slashes removed, which is the form the pages name as their canonical URL. + /// + /// A path of nothing but slashes collapses to the root, so `//` redirects to `/` while `/` itself is left alone. + /// - Parameter path: the requested path. + /// - Returns: the canonical form of the path. + func canonicalPath( + of path: String + ) -> String { + var canonical = path + + while canonical.hasSuffix(.pathSeparator), canonical != .pathSeparator { + canonical.removeLast() + } + + return canonical + } + + /// The query the redirect preserves, so a campaign-tagged link survives the canonicalization. + /// - Parameter request: the incoming request. + /// - Returns: the query prefixed with its delimiter, or an empty string when the request carries none. + func query( + of request: Request + ) -> String { + guard + let query = request.uri.query, + !query.isEmpty + else { + return "" + } + + return .querySeparator + query + } + +} + +// MARK: - String+Constants + +private extension String { + /// The separator a canonical path never ends on, and the root path it collapses to. + static let pathSeparator = "/" + /// The delimiter placed between a redirect target's path and its query. + static let querySeparator = "?" +} diff --git a/Packages/Infrastructure/Tests/Cases/Public/Middlewares/TrailingSlashRedirectMiddlewareTests.swift b/Packages/Infrastructure/Tests/Cases/Public/Middlewares/TrailingSlashRedirectMiddlewareTests.swift new file mode 100644 index 0000000..7b053bf --- /dev/null +++ b/Packages/Infrastructure/Tests/Cases/Public/Middlewares/TrailingSlashRedirectMiddlewareTests.swift @@ -0,0 +1,150 @@ +import HTTPTypes +import Hummingbird +import HummingbirdTesting +import Testing + +@testable import Infrastructure + +@Suite( + "TrailingSlashRedirectMiddleware middleware", + .tags(.middleware) +) +struct TrailingSlashRedirectMiddlewareTests { + + // MARK: Functional tests + + @Test + func `redirects a path carrying a trailing slash`() async throws { + try await app().test(.router) { client in + try await client.execute( + uri: "/hello/", + method: .get + ) { response in + #expect(response.status == .movedPermanently) + #expect(response.headers[.location] == "/hello") + } + } + } + + @Test + func `preserves the query of the redirected request`() async throws { + try await app().test(.router) { client in + try await client.execute( + uri: "/hello/?utm_source=test&utm_medium=email", + method: .get + ) { response in + #expect(response.headers[.location] == "/hello?utm_source=test&utm_medium=email") + } + } + } + + @Test + func `collapses a path of nothing but slashes onto the root`() async throws { + try await app().test(.router) { client in + try await client.execute( + uri: "//", + method: .get + ) { response in + #expect(response.status == .movedPermanently) + #expect(response.headers[.location] == "/") + } + } + } + + @Test + func `strips every trailing slash at once`() async throws { + try await app().test(.router) { client in + try await client.execute( + uri: "/hello///", + method: .get + ) { response in + #expect(response.headers[.location] == "/hello") + } + } + } + + @Test + func `passes the canonical path through`() async throws { + try await app().test(.router) { client in + try await client.execute( + uri: "/hello", + method: .get + ) { response in + #expect(response.status == .ok) + #expect(response.headers[.location] == nil) + } + } + } + + @Test + func `leaves the root path alone`() async throws { + try await app().test(.router) { client in + try await client.execute( + uri: "/", + method: .get + ) { response in + #expect(response.status == .ok) + #expect(response.headers[.location] == nil) + } + } + } + + @Test + func `redirects a head request as it does a get`() async throws { + try await app().test(.router) { client in + try await client.execute( + uri: "/hello/", + method: .head + ) { response in + #expect(response.status == .movedPermanently) + #expect(response.headers[.location] == "/hello") + } + } + } + + @Test + func `passes a post through so its body survives`() async throws { + try await app().test(.router) { client in + try await client.execute( + uri: "/hello/", + method: .post + ) { response in + #expect(response.status == .ok) + #expect(response.headers[.location] == nil) + } + } + } + +} + +// MARK: - Helpers + +private extension TrailingSlashRedirectMiddlewareTests { + + // MARK: Methods + + /// Builds an application whose router applies the trailing-slash middleware ahead of a `/hello` + /// route answering both `GET` and `POST`, and a root route standing in for the landing page. + func app() -> some ApplicationProtocol { + let router = Router() + + router.addMiddleware { + TrailingSlashRedirectMiddleware() + } + + router.get("hello") { _, _ in + "Hello!" + } + + router.post("hello") { _, _ in + "Posted!" + } + + router.get("/") { _, _ in + "Root!" + } + + return Application(router: router) + } + +} diff --git a/Services/Website/README.md b/Services/Website/README.md index ca0d288..3382af1 100644 --- a/Services/Website/README.md +++ b/Services/Website/README.md @@ -107,6 +107,24 @@ The groups are matched in order, so the specific media types (`text/css`, `text/ | `http.port` | `HTTP_PORT` | _none_ | Port the server listens on. Supplied via the `--http-port` CLI flag (the Docker image passes `8080`). | | `http.serverName` | `HTTP_SERVER_NAME` | `SiteWebsite` | Server name and logger label. | +### HTTPS redirect +| Config key | Environment variable | Default | Description | +| --- | --- | --- | --- | +| `https.trustForwardedProto` | `HTTPS_TRUST_FORWARDED_PROTO` | `false` | Read the visitor's scheme from the `X-Forwarded-Proto` header and answer the plain-HTTP ones with `301 Moved Permanently` to the same path on `site.origin`. Enable **only** behind a reverse proxy that sets the header — it is the sole trigger. | + +Redirecting collapses the `http://` and `https://` copies of every page onto one address, which is what a search engine consolidates a site's signals against. Three details: + +- **`301`, not `302`** — a temporary redirect keeps the HTTP URLs indexed. Browsers cache it for a long time, so settle the target first. +- **Target built from `site.origin`, not the `Host` header** — a client cannot steer it. An origin that is not itself HTTPS disables the middleware instead of looping. +- **`/.well-known/` is exempt** — redirecting the ACME challenge path breaks certificate renewal. + +`docker-compose.yml` enables it for production; `docker-compose.override.yml` pins it off for local development. + +Trailing slashes are canonicalized separately and unconditionally, with no configuration key: the router matches `/mr-rock` and `/mr-rock/` alike, so +every `GET`/`HEAD` whose path ends in a slash is answered with a `301` to the form without one (`//` collapses to `/`; `/` is left alone). The +`Location` is relative, so it keeps the request's own scheme and host. Other methods pass through, since a client may repeat a redirected `POST` as a +`GET` and drop the body. + ### Logging | Config key | Environment variable | Default | Description | | --- | --- | --- | --- | @@ -169,7 +187,7 @@ The tracker's origin is not a configuration key: it is single-sourced in code so | `security.permissionsPolicy` | `SECURITY_PERMISSIONS_POLICY` | `accelerometer=(), camera=(), geolocation=(), gyroscope=(), magnetometer=(), microphone=(), payment=(), usb=()` | | `security.strictTransportSecurity` | `SECURITY_STRICT_TRANSPORT_SECURITY` | _none (omitted)_ | -`Strict-Transport-Security` has **no default** and is omitted unless explicitly configured: browsers ignore it on plain HTTP but remember it stickily once seen, so it must stay off in local HTTP development. `docker-compose.yml` enables it for production, where it takes effect once traffic is served over HTTPS behind a TLS-terminating proxy. +`Strict-Transport-Security` has **no default** and is omitted unless explicitly configured: browsers ignore it on plain HTTP but remember it stickily once seen, so it must stay off in local HTTP development. `docker-compose.yml` enables it for production, where it takes effect once traffic is served over HTTPS behind a TLS-terminating proxy. [`https.trustForwardedProto`](#https-redirect) carries the same caveat: a cached `301` is as sticky as an HSTS commitment. ## Running locally Directly with Swift: diff --git a/Services/Website/Sources/App/Extensions/App+Build.swift b/Services/Website/Sources/App/Extensions/App+Build.swift index a46ce9f..e75d04f 100644 --- a/Services/Website/Sources/App/Extensions/App+Build.swift +++ b/Services/Website/Sources/App/Extensions/App+Build.swift @@ -136,7 +136,8 @@ private func logger( /// Builds the application's router. /// /// Registers the request-logging middleware, the security-headers middleware that stamps the given `securityHeaders` onto every response, the -/// HTTPS-redirect middleware that bounces requests forwarded over plain HTTP to the canonical origin, the vary middleware that marks every response as varying on `Accept-Encoding`, the response-compression middleware that compresses responses +/// HTTPS-redirect middleware that bounces requests forwarded over plain HTTP to the canonical origin, the trailing-slash redirect middleware that +/// collapses each path onto its canonical form, the vary middleware that marks every response as varying on `Accept-Encoding`, the response-compression middleware that compresses responses /// larger than `minimumResponseSizeToCompress` when the client advertises support, the localization middleware that negotiates the request's /// language from its `Accept-Language` header, the not-found middleware that serves the error page, and the static file middleware that serves the /// contents of `staticFilesPath` (tagging responses with the given `cacheControl` directives), then adds the `RootController` routes that @@ -144,7 +145,8 @@ private func logger( /// /// The security-headers middleware sits just inside request logging so it covers every response that reaches a client — the landing page, the compressed /// responses, the rendered not-found page, and the served static files. The HTTPS redirect sits directly beneath it, so a redirect carries the security -/// headers but skips the negotiation, compression, and file lookup it would otherwise pay for. +/// headers but skips the negotiation, compression, and file lookup it would otherwise pay for. The trailing-slash redirect follows it, ahead of the +/// routes and `FileMiddleware` that would otherwise answer both spellings of every path. /// - Parameters: /// - staticFilesPath: the folder, relative to the working directory, the static files are served from. /// - assetVersion: the version token the pages append to their asset URLs, or `nil` to leave them unversioned. @@ -184,6 +186,7 @@ private func router( HTTPSRedirectMiddleware( configuration: httpsRedirect ) + TrailingSlashRedirectMiddleware() VaryMiddleware() ResponseCompressionMiddleware( minimumResponseSizeToCompress: compressionMinResponseSize diff --git a/Services/Website/docker-compose.override.yml b/Services/Website/docker-compose.override.yml index 80389f0..2d69ace 100644 --- a/Services/Website/docker-compose.override.yml +++ b/Services/Website/docker-compose.override.yml @@ -18,6 +18,7 @@ services: dockerfile: Services/Website/Dockerfile environment: LOG_LEVEL: debug + HTTPS_TRUST_FORWARDED_PROTO: "false" DATABASE_DRIVER: ${DATABASE_DRIVER:-inMemory} DATABASE_HOST: postgres DATABASE_TLS: ${DATABASE_TLS:-off} diff --git a/Services/Website/docker-compose.yml b/Services/Website/docker-compose.yml index 25773e3..31d4721 100644 --- a/Services/Website/docker-compose.yml +++ b/Services/Website/docker-compose.yml @@ -19,12 +19,9 @@ services: environment: LOG_LEVEL: ${LOG_LEVEL:-info} HTTP_SERVER_NAME: ${HTTP_SERVER_NAME:-SiteWebsite} - SECURITY_STRICT_TRANSPORT_SECURITY: "${SECURITY_STRICT_TRANSPORT_SECURITY:-max-age=31536000; includeSubDomains}" - # Falls back to the policy the app ships with; set it in `.env` to allow the analytics origin, - # which must match `String.Analytics.origin`. SECURITY_CONTENT_SECURITY_POLICY: "${SECURITY_CONTENT_SECURITY_POLICY:-default-src 'self'; object-src 'none'; base-uri 'self'; frame-ancestors 'none'}" - # Persistence: a managed PostgreSQL database. Provide the password via the environment or a secret — never - # commit it. + SECURITY_STRICT_TRANSPORT_SECURITY: "${SECURITY_STRICT_TRANSPORT_SECURITY:-max-age=31536000; includeSubDomains}" + HTTPS_TRUST_FORWARDED_PROTO: "${HTTPS_TRUST_FORWARDED_PROTO:-true}" DATABASE_DRIVER: ${DATABASE_DRIVER:-postgres} DATABASE_HOST: ${DATABASE_HOST:?DATABASE_HOST is required} DATABASE_PORT: ${DATABASE_PORT:-5432}