fix: url match logic of http server (denoland#199)

ry · Feb 18, 2019 · da188a7 · da188a7
1 parent 3be908f
commit da188a7
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 20 deletions.
diff --git a/http/server.ts b/http/server.ts
@@ -152,25 +152,15 @@ class HttpServerImpl implements HttpServer {
 
   async listen(addr: string, cancel: Deferred = defer()) {
     for await (const { req, res } of serve(addr, cancel)) {
-      let lastMatch: RegExpMatchArray;
-      let lastHandler: HttpHandler;
-      for (const { pattern, handler } of this.handlers) {
-        const match = req.url.match(pattern);
-        if (!match) {
-          continue;
-        }
-        if (!lastMatch) {
-          lastMatch = match;
-          lastHandler = handler;
-        } else if (match[0].length > lastMatch[0].length) {
-          // use longest match
-          lastMatch = match;
-          lastHandler = handler;
-        }
-      }
-      req.match = lastMatch;
-      if (lastHandler) {
-        await lastHandler(req, res);
+      let { pathname } = new URL(req.url, addr);
+      const { index, match } = findLongestAndNearestMatch(
+        pathname,
+        this.handlers.map(v => v.pattern)
+      );
+      req.match = match;
+      if (index > -1) {
+        const { handler } = this.handlers[index];
+        await handler(req, res);
         if (!res.isResponded) {
           await res.respond({
             status: 500,
@@ -187,6 +177,36 @@ class HttpServerImpl implements HttpServer {
   }
 }
 
+/**
+ * Find the match that appeared in the nearest position to the beginning of word.
+ * If positions are same, the longest one will be picked.
+ * Return -1 and null if no match found.
+ * */
+export function findLongestAndNearestMatch(
+  pathname: string,
+  patterns: (string | RegExp)[]
+): { index: number; match: RegExpMatchArray } {
+  let lastMatchIndex = pathname.length;
+  let lastMatchLength = 0;
+  let match: RegExpMatchArray = null;
+  let index = -1;
+  for (let i = 0; i < patterns.length; i++) {
+    const pattern = patterns[i];
+    const m = pathname.match(pattern);
+    if (!m) continue;
+    if (
+      m.index < lastMatchIndex ||
+      (m.index === lastMatchIndex && m[0].length > lastMatchLength)
+    ) {
+      index = i;
+      match = m;
+      lastMatchIndex = m.index;
+      lastMatchLength = m[0].length;
+    }
+  }
+  return { index, match };
+}
+
 class ServerResponderImpl implements ServerResponder {
   constructor(private w: Writer) {}
 

diff --git a/http/server_test.ts b/http/server_test.ts
@@ -6,10 +6,11 @@
 // https://github.com/golang/go/blob/master/src/net/http/responsewrite_test.go
 
 import { Buffer, copy, Reader } from "deno";
-import { assert, assertEqual, test } from "../testing/mod.ts";
+import { assert, assertEqual, runTests, test } from "../testing/mod.ts";
 import {
   createResponder,
   createServer,
+  findLongestAndNearestMatch,
   readRequest,
   readResponse,
   ServerResponse,
@@ -102,6 +103,49 @@ test(async function httpReadRequestChunkedBody() {
   assert.equal(dest.toString(), "deno.land");
 });
 
+test(function httpMatchNearest() {
+  assert.equal(
+    findLongestAndNearestMatch("/foo", ["/foo", "/bar", "/f"]).index,
+    0
+  );
+  assert.equal(
+    findLongestAndNearestMatch("/foo", ["/foo", "/foo/bar"]).index,
+    0
+  );
+  assert.equal(
+    findLongestAndNearestMatch("/foo/bar", [
+      "/",
+      "/foo",
+      "/hoo",
+      "/hoo/foo/bar",
+      "/foo/bar"
+    ]).index,
+    4
+  );
+  assert.equal(
+    findLongestAndNearestMatch("/foo/bar/foo", ["/foo", "/foo/bar", "/bar/foo"])
+      .index,
+    1
+  );
+  assert.equal(
+    findLongestAndNearestMatch("/foo", ["/", "/hoo", "/hoo/foo"]).index,
+    0
+  );
+  assert.equal(
+    findLongestAndNearestMatch("/deno/land", [/d(.+?)o/, /d(.+?)d/]).index,
+    1
+  );
+  assert.equal(findLongestAndNearestMatch("/foo", ["/", "/a/foo"]).index, 0);
+  assert.equal(
+    findLongestAndNearestMatch("/foo", [/\/foo/, /\/bar\/foo/]).index,
+    0
+  );
+  assert.equal(
+    findLongestAndNearestMatch("/foo", [/\/a\/foo/, /\/foo/]).index,
+    1
+  );
+});
+
 test(async function httpServer() {
   const server = createServer();
   server.handle("/index", async (req, res) => {