Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the HttpRouter to handle overlapping of the routes #359

Merged
merged 1 commit into from
Jan 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 64 additions & 35 deletions Sources/HttpRouter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -81,47 +81,76 @@ open class HttpRouter {
}

private func findHandler(_ node: inout Node, params: inout [String: String], generator: inout IndexingIterator<[String]>) -> ((HttpRequest) -> HttpResponse)? {
guard let pathToken = generator.next()?.removingPercentEncoding else {
// if it's the last element of the requested URL, check if there is a pattern with variable tail.
if let variableNode = node.nodes.filter({ $0.0.first == ":" }).first {
if variableNode.value.nodes.isEmpty {
params[variableNode.0] = ""
return variableNode.value.handler

var matchedRoutes = [Node]()
findHandler(&node, params: &params, generator: &generator, matchedNodes: &matchedRoutes, index: 0, count: generator.reversed().count)
return matchedRoutes.first?.handler
}

/// Find the handlers for a specified route
///
/// - Parameters:
/// - node: The root node of the tree representing all the routes
/// - params: The parameters of the match
/// - generator: The IndexingIterator to iterate through the pattern to match
/// - matchedNodes: An array with the nodes matching the route
/// - index: The index of current position in the generator
/// - count: The number of elements if the route to match
private func findHandler(_ node: inout Node, params: inout [String: String], generator: inout IndexingIterator<[String]>, matchedNodes: inout [Node], index: Int, count: Int) {

if let pathToken = generator.next()?.removingPercentEncoding {

var currentIndex = index + 1
let variableNodes = node.nodes.filter { $0.0.first == ":" }
if let variableNode = variableNodes.first {
if variableNode.1.nodes.count == 0 {
// if it's the last element of the pattern and it's a variable, stop the search and
// append a tail as a value for the variable.
let tail = generator.joined(separator: "/")
if tail.count > 0 {
params[variableNode.0] = pathToken + "/" + tail
} else {
params[variableNode.0] = pathToken
}

matchedNodes.append(variableNode.value)
return
}
params[variableNode.0] = pathToken
findHandler(&node.nodes[variableNode.0]!, params: &params, generator: &generator, matchedNodes: &matchedNodes, index: currentIndex, count: count)
}
return node.handler
}
let variableNodes = node.nodes.filter { $0.0.first == ":" }
if let variableNode = variableNodes.first {
if variableNode.1.nodes.count == 0 {
// if it's the last element of the pattern and it's a variable, stop the search and
// append a tail as a value for the variable.
let tail = generator.joined(separator: "/")
if tail.count > 0 {
params[variableNode.0] = pathToken + "/" + tail
} else {
params[variableNode.0] = pathToken
}
return variableNode.1.handler

if var node = node.nodes[pathToken] {
findHandler(&node, params: &params, generator: &generator, matchedNodes: &matchedNodes, index: currentIndex, count: count)
}
params[variableNode.0] = pathToken
return findHandler(&node.nodes[variableNode.0]!, params: &params, generator: &generator)
}
if var node = node.nodes[pathToken] {
return findHandler(&node, params: &params, generator: &generator)
}
if var node = node.nodes["*"] {
return findHandler(&node, params: &params, generator: &generator)
}
if let startStarNode = node.nodes["**"] {
let startStarNodeKeys = startStarNode.nodes.keys
while let pathToken = generator.next() {
if startStarNodeKeys.contains(pathToken) {
return findHandler(&startStarNode.nodes[pathToken]!, params: &params, generator: &generator)

if var node = node.nodes["*"] {
findHandler(&node, params: &params, generator: &generator, matchedNodes: &matchedNodes, index: currentIndex, count: count)
}

if let startStarNode = node.nodes["**"] {
let startStarNodeKeys = startStarNode.nodes.keys
while let pathToken = generator.next() {
currentIndex += 1
if startStarNodeKeys.contains(pathToken) {
findHandler(&startStarNode.nodes[pathToken]!, params: &params, generator: &generator, matchedNodes: &matchedNodes, index: currentIndex, count: count)
}
}
}
} else if let variableNode = node.nodes.filter({ $0.0.first == ":" }).first {
// if it's the last element of the requested URL, check if there is a pattern with variable tail.
if variableNode.value.nodes.isEmpty {
params[variableNode.0] = ""
matchedNodes.append(variableNode.value)
return
}
}

// if it's the last element and the path to match is done then it's a pattern matching
if node.nodes.isEmpty && index == count {
matchedNodes.append(node)
return
}
return nil
}

private func stripQuery(_ path: String) -> String {
Expand Down
2 changes: 2 additions & 0 deletions XCode/Swifter.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
7AE893EA1C05127900A29F63 /* SwifteriOS.h in Headers */ = {isa = PBXBuildFile; fileRef = 7AE893E91C05127900A29F63 /* SwifteriOS.h */; settings = {ATTRIBUTES = (Public, ); }; };
7AE893FE1C0512C400A29F63 /* SwifterMac.h in Headers */ = {isa = PBXBuildFile; fileRef = 7AE893FD1C0512C400A29F63 /* SwifterMac.h */; settings = {ATTRIBUTES = (Public, ); }; };
7AE8940D1C05151100A29F63 /* Launch Screen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 7AE8940C1C05151100A29F63 /* Launch Screen.storyboard */; };
7B11AD4B21C9A8A6002F8820 /* SwifterTestsHttpRouter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7CCB8C5F1D97B8CC008B9712 /* SwifterTestsHttpRouter.swift */; };
7B74CFA82163C40F001BE07B /* AppDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7CDAB80C1BE2A1D400C8A977 /* AppDelegate.swift */; };
7C377E181D964B96009C6148 /* String+File.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C377E161D964B6A009C6148 /* String+File.swift */; };
7C377E191D964B9F009C6148 /* String+File.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C377E161D964B6A009C6148 /* String+File.swift */; };
Expand Down Expand Up @@ -780,6 +781,7 @@
0858E7F41D68BB2600491CD1 /* IOSafetyTests.swift in Sources */,
7C4785E91C71D15600A9FE73 /* SwifterTestsWebSocketSession.swift in Sources */,
7CCD87721C660B250068099B /* SwifterTestsStringExtensions.swift in Sources */,
7B11AD4B21C9A8A6002F8820 /* SwifterTestsHttpRouter.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down
38 changes: 37 additions & 1 deletion XCode/SwifterTestsCommon/SwifterTestsHttpRouter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//

import XCTest
import Swifter

class SwifterTestsHttpRouter: XCTestCase {

Expand Down Expand Up @@ -105,7 +106,7 @@ class SwifterTestsHttpRouter: XCTestCase {
XCTAssertEqual(router.route(nil, path: "/a/b/")?.0[":var"], "")
}

func testHttpRouterPercentEnocedPathSegments() {
func testHttpRouterPercentEncodedPathSegments() {

let router = HttpRouter()

Expand All @@ -117,4 +118,39 @@ class SwifterTestsHttpRouter: XCTestCase {
XCTAssertNotNil(router.route(nil, path: "/a/%3C%3E/%5E"))
}

func testHttpRouterHandlesOverlappingPaths() {

let router = HttpRouter()
let request = HttpRequest()

let staticRouteExpectation = expectation(description: "Static Route")
var foundStaticRoute = false
router.register("GET", path: "a/b") { _ in
foundStaticRoute = true
staticRouteExpectation.fulfill()
return HttpResponse.accepted
}

let variableRouteExpectation = expectation(description: "Variable Route")
var foundVariableRoute = false
router.register("GET", path: "a/:id/c") { _ in
foundVariableRoute = true
variableRouteExpectation.fulfill()
return HttpResponse.accepted
}

let staticRouteResult = router.route("GET", path: "a/b")
let staticRouterHandler = staticRouteResult?.1
XCTAssertNotNil(staticRouteResult)
_ = staticRouterHandler?(request)

let variableRouteResult = router.route("GET", path: "a/b/c")
let variableRouterHandler = variableRouteResult?.1
XCTAssertNotNil(variableRouteResult)
_ = variableRouterHandler?(request)

waitForExpectations(timeout: 10, handler: nil)
XCTAssertTrue(foundStaticRoute)
XCTAssertTrue(foundVariableRoute)
}
}