Patch IncomingMessage.url in mounted apps

Previously they would get the full request URL. The request URL is
now preserved in `originalURL`, and a mounted app receives relative
URLs.
E.g. sth mounted at "/admin" would get "/view" for "/admin/view".

Matches Express.js.

Author: helje5

Package.swift

@@ -18,8 +18,7 @@ let package = Package(
   ],
 
   dependencies: [
-    .package(url: "https://github.com/Macro-swift/Macro.git",
-             from: "1.0.16"),
+    .package(url: "https://github.com/Macro-swift/Macro.git", from: "1.0.20"),
     .package(url: "https://github.com/AlwaysRightInstitute/mustache.git",
              from: "1.0.2")
   ],

Sources/express/Express.swift

@@ -337,6 +337,11 @@ enum ExpressExtKey {
     static let loggingKey   = "baseurl"
   }
 
+  enum OriginalURL: EnvironmentKey {
+    static let defaultValue : String? = nil
+    static let loggingKey   = "originalurl"
+  }
+
   /**
    * Request parameters.
    *

Sources/express/IncomingMessage.swift

@@ -92,8 +92,8 @@ public extension IncomingMessage {
     guard let qIdx = url.firstIndex(of: "?") else {
       return url.isEmpty ? "/" : url
     }
-    let p = String(url[..<qIdx])
-    return p.isEmpty ? "/" : p
+    if url.startIndex == qIdx { return "/" } // empty
+    return String(url[..<qIdx])
   }
 
   /// A reference to the active application. Updated when subapps are triggered.
@@ -144,13 +144,26 @@ public extension IncomingMessage {
   }
 
   /**
-   * Contains the part of the URL which matched the current route. Example:
+   * The original URL as received from the client, before
+   * any URL rewriting by mounted middleware.
+   *
+   * Unlike ``url``, this is never modified by routing.
+   * Falls back to ``url`` if not yet set by Express.
+   */
+  var originalURL : String {
+    get { environment[ExpressExtKey.OriginalURL.self] ?? url }
+    set { environment[ExpressExtKey.OriginalURL.self] = newValue }
+  }
+
+  /**
+   * Contains the part of the URL which matched the current
+   * route. Example:
    * ```
    * app.get("/admin/index") { ... }
    * ```
    *
-   * when this is invoked with "/admin/index/hello/world", the baseURL will
-   * be "/admin/index".
+   * when this is invoked with "/admin/index/hello/world",
+   * the baseURL will be "/admin/index".
    */
   var baseURL : String? {
     set { environment[ExpressExtKey.BaseURL.self] = newValue }

Sources/express/Route.swift

@@ -157,7 +157,12 @@ open class Route: MiddlewareObject, ErrorMiddlewareObject, RouteKeeper,
   {
     let ids = debug ? logPrefix : ""
     if debug { console.log("\(ids) > enter route:", self) }
-    
+
+    // Capture the original URL before any rewriting
+    if req.environment[ExpressExtKey.OriginalURL.self] == nil {
+      req.environment[ExpressExtKey.OriginalURL.self] = req.url
+    }
+
     if let methods = self.methods, !methods.isEmpty {
       guard case .request(let head) = req.head,
             methods.contains(head.method) else {
@@ -169,59 +174,35 @@ open class Route: MiddlewareObject, ErrorMiddlewareObject, RouteKeeper,
       }
     }
 
-    // FIXME: Could also be a full URL! (CONNECT)
+    // TBD: Could also be a full URL! (CONNECT)
+    // req.url is relative to the current mount point
     let reqPath = req.url.isEmpty ? "/" : {
-      // Strip of query parameters and such. This is the raw URL,
-      // but we need to match just the path.
       let s = req.url
       if let idx = s.firstIndex(where: { $0 == "#" || $0 == "?" }) {
         return String(s[..<idx])
       }
-      else {
-        return s
-      }
+      else { return s }
     }()
 
     let params    : IncomingMessage.Params
     let matchPath : String?
-    if let pattern = urlPattern { // this route has a path pattern assigned
-      var newParams = req.params // TBD
-      
-      if let base = req.baseURL {
-        let mountPath = String(reqPath[base.endIndex..<reqPath.endIndex])
-        let comps     = split(urlPath: mountPath)
+    if let pattern = urlPattern {
+      var newParams = req.params
+      let comps = split(urlPath: reqPath)
 
-        let mountMatchPath = RoutePattern.match(pattern : pattern,
-                                                against : comps,
-                                                exact   : exact,
-                                                variables : &newParams)
-        guard let match = mountMatchPath else {
-          if debug {
-            console.log("\(ids) mount route path does not match, next:", self)
-          }
-          if let error = error { throw error }
-          return upperNext()
+      guard let mp = RoutePattern.match(pattern: pattern, against: comps,
+                                        exact: exact, variables: &newParams)
+       else {
+        if debug {
+          console.log(
+            "\(ids) route path does not match, next:",
+            self)
         }
-        
-        matchPath = base + match
-      }
-      else {
-        let comps = split(urlPath: reqPath)
-        
-        guard let mp = RoutePattern.match(pattern   : pattern,
-                                          against   : comps,
-                                          exact     : exact,
-                                          variables : &newParams)
-         else {
-          if debug {
-            console.log("\(ids) route path does not match, next:", self)
-          }
-          if let error = error { throw error }
-          return upperNext()
-         }
-        matchPath = mp
+        if let error = error { throw error }
+        return upperNext()
       }
-      
+      matchPath = mp
+
       if debug { console.log("\(ids)     path match:", matchPath) }
 
       params = newParams
@@ -235,18 +216,31 @@ open class Route: MiddlewareObject, ErrorMiddlewareObject, RouteKeeper,
       guard !self.errorMiddleware.isEmpty else { throw error }
     }
     else {
-      guard !self.middleware     .isEmpty else { return upperNext() }
+      guard !self.middleware.isEmpty else { return upperNext() }
     }
 
     // push route state
     let oldParams = req.params
     let oldRoute  = req.route
     let oldBase   = req.baseURL
-    req.params  = params
-    req.route   = self
+    let oldUrl    = req.url
+    req.params = params
+    req.route  = self
     if let mp = matchPath {
-      req.baseURL = mp
-      if debug { console.log("\(ids)   push baseURL:", req.baseURL) }
+      req.baseURL = (oldBase ?? "") + mp
+      if !exact {
+        // Rewrite req.url like Node.js Express:
+        // strip the matched prefix, keep query/fragment.
+        var newUrl = String(req.url.dropFirst(mp.count))
+        if newUrl.isEmpty { newUrl = "/" }
+        else if newUrl.first == "?" || newUrl.first == "#" {
+          newUrl = "/" + newUrl
+        }
+        req.url = newUrl
+      }
+      if debug {
+        console.log("\(ids)   push baseURL:", req.baseURL, "url:", req.url)
+      }
     }
 
     final class MiddlewareWalker {
@@ -340,6 +334,7 @@ open class Route: MiddlewareObject, ErrorMiddlewareObject, RouteKeeper,
       req.params  = oldParams
       req.route   = oldRoute
       req.baseURL = oldBase
+      req.url     = oldUrl
 
       if let arg = args.first { // lame 1-object spread to pass on errors
         upperNext(arg)

Tests/RouteTests/RouteMountingTests.swift

@@ -1,6 +1,6 @@
 import XCTest
 import MacroTestUtilities
-import class     http.IncomingMessage
+import http // IncomingMessage
 @testable import express
 
 final class RouteMountingTests: XCTestCase {
@@ -13,15 +13,15 @@ final class RouteMountingTests: XCTestCase {
     outerRoute.route("/admin")
       .get("/view") { req, res, next in
         XCTAssertEqual(req.baseURL, "/admin/view", "baseURL does not match")
-        XCTAssertEqual(req.url,     "/admin/view", "HTTP URL does not match")
+        XCTAssertEqual(req.url,     "/view", "url should be relative to mount")
+        XCTAssertEqual(req.originalURL, "/admin/view",
+                       "originalURL should be the full path")
         didCallRoute = true
       }
 
     // test
 
-    let req = IncomingMessage(
-      .init(version: .init(major: 1, minor: 1), method: .GET,
-            uri: "/admin/view"))
+    let req = IncomingMessage(url: "/admin/view")
     let res = TestServerResponse()
 
     var didCallNext = false
@@ -46,7 +46,10 @@ final class RouteMountingTests: XCTestCase {
     outerRoute.route(id: "outer", "/admin")
       .get(id: "error", "/view") { error, req, res, next in
         XCTAssertEqual(req.baseURL, "/admin/view", "baseURL does not match")
-        XCTAssertEqual(req.url,     "/admin/view", "HTTP URL does not match")
+        XCTAssertEqual(req.url,     "/view?answer=42",
+                       "url should be relative to mount")
+        XCTAssertEqual(req.originalURL, "/admin/view?answer=42",
+                       "originalURL should be the full path")
         didCallErrorMiddleware = true
       }
       .use(id: "thrower") { req, res, next in
@@ -56,9 +59,7 @@ final class RouteMountingTests: XCTestCase {
 
     // test
 
-    let req = IncomingMessage(
-      .init(version: .init(major: 1, minor: 1), method: .GET,
-            uri: "/admin/view"))
+    let req = IncomingMessage(url: "/admin/view?answer=42"))
     let res = TestServerResponse()
 
     var didCallNext = false