Strange Wildcard behavior

[christopher-kleine]

Hi,

while playing around with this cool router, I noticed something quite strange. Given the following source:

package main

import (
    "fmt"
    "net/http"

    "github.com/husobee/vestigo"
)

func main() {
    router := vestigo.NewRouter()

    router.Get("/greet/:name", GreetUser)
    router.HandleFunc("/*", IndexHandler)

    http.ListenAndServe(":8080", router)
}

func GreetUser(w http.ResponseWriter, r *http.Request) {
    fmt.Fprintf(w, "Welcome %s\n", vestigo.Param(r, "name"))
}

func IndexHandler(w http.ResponseWriter, r *http.Request) {
    fmt.Fprintf(w, "You opened %s\n", vestigo.Param(r, "_name"))
}

The result looks like this:

/greet/ -> 404 (Expected)
/greet/demo -> Welcome demo (Expected)
/demo   -> You opened demo (Expected)
/demo/  -> You opened demo/ (Expected)
/gre    -> Welcome gre (Unexpected)

Is it just me who finds this strange?

[husobee]

Yes, that is strange, and very unexpected. Will try to debug this tonight.

[husobee]

If you wouldn't mind, please verify this PR: https://github.com/husobee/vestigo/pull/50 corrects the issue you were seeing? I added a test to the router tests with what you experienced, and it passes now.

[christopher-kleine]

Hi,

it seems I still get the same strange behavior. What is even more strange (and I didn't notice this at first), is that the first letter determines if it matches or not. The GreetUser route is called, even if the request is /gasdadasdfsfsdf

Thanks for your time and effort. So far I like this router the best.

[husobee]

Are you using the branch "issue49"? I haven't merged the fix to master yet. Will look into further in a bit.

Thanks

On January 10, 2017 5:57:06 PM EST, Chris notifications@github.com wrote:

Hi,

it seems I still get the same strange behavior. What is even more strange (and I didn't notice this at first), is that the first letter determines if it matches or not. The GreetUser route is called, even if the request is /gasdadasdfsfsdf

Thanks for your time and effort. So far I like this router the best.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/husobee/vestigo/issues/49#issuecomment-271724936

[christopher-kleine]

Yes, I did. To make sure everything is up-to-date, I deleted my binary, deleted the former Vestigo, downloaded the new Vestigo and made sure the code is the same as in the commit.

After that, I tried it again. Same result.

I can give you a "slightly" more complex Test I've been running. The result might cause you a slight headache.

package main

import (
    "fmt"
    "net/http"

    "github.com/husobee/vestigo"
)

func main() {
    router := vestigo.NewRouter()

    // Handle all other requests
    router.Get("/*", ServeIndex)

    // Userlist
    router.Get("/users/", UserIndex)
    router.Get("/users/:name", UserShow)

    // Handle books
    router.Get("/books/", BookIndex)
    router.Get("/books/:book", BookShow)

    // Handle bookcase
    router.Get("/bookcase/", BookcaseIndex)
    router.Get("/bookcase/:book", BookcaseShow)
    router.Get("/bookcase/:book/bookmarks", BookmarksIndex)

    http.ListenAndServe(":8080", router)
}

func FileExists(Filename string) bool {
    return Filename == "./static/css/demo.css" 
}

func ServeIndex(w http.ResponseWriter, r *http.Request) {
    var Filename string

    Filename = vestigo.Param(r, "_name")

    if Filename == "" {
        fmt.Fprintln(w, "Sending index,html")
    } else {
        if FileExists("./static/" + Filename) {
            fmt.Fprintf(w, "Sending %s\n", "./static/" + Filename)
        } else {
            fmt.Fprintf(w, "File %s was not found.\n", "./static/" + Filename)
        }
    }
}

func BookIndex(w http.ResponseWriter, r *http.Request) {
    fmt.Fprintln(w, "Here's our list of books ...")
}

func BookShow(w http.ResponseWriter, r *http.Request) {
    fmt.Fprintf(w, "Showing a book %s\n", vestigo.Param(r, "book"))
}

func BookcaseIndex(w http.ResponseWriter, r *http.Request) {
    fmt.Fprintln(w, "Here's your bookcase")
}

func BookcaseShow(w http.ResponseWriter, r *http.Request) {
    fmt.Fprintf(w, "Here is your book %s\n", vestigo.Param(r, "book"))
}

func BookmarksIndex(w http.ResponseWriter, r *http.Request) {
    fmt.Fprintf(w, "Your bookmarks for your book %s\n", vestigo.Param(r, "book"))
}

func UserIndex(w http.ResponseWriter, r *http.Request) {
    fmt.Fprintln(w, "All users ...")
}

func UserShow(w http.ResponseWriter, r *http.Request) {
    fmt.Fprintf(w, "You're watching at the profile of user %s\n", vestigo.Param(r, "name"))
}

Request	Answer	Expected?
/	Sending index,html	Yes
/css/demo.css	Sending ./static/css/demo.css	Yes
/js/demo.js	File ./static/js/demo.js was not found.	Yes
/b	Not Found	No
/books	Showing a book s	No
/books/	Here's our list of books ...	Yes
/books/sonea	Showing a book sonea	Yes
/books/sonea/	Not Found	Yes
/bookcase	Here is your book case	No
/bookcase/	Here's your bookcase	Yes
/bookcase/sonea	Here is your book sonea	Yes
/bookcase/sonea/	Not Found	Yes
/bookcase/sonea/b	Not Found	Yes
/bookcase/sonea/bookmarks	Your bookmarks for your book sonea	Yes

[husobee]

Thanks, will review.

On January 10, 2017 6:28:23 PM EST, Chris notifications@github.com wrote:

Yes, I did. To make sure everything is up-to-date, I deleted my binary, deleted the former Vestigo, downloaded the new Vestigo and made sure the code is the same as in the commit.

After that, I tried it again. Same result.

I can give you a "slightly" more complex Test I've been running. The result might cause you a slight headache.

package main

import (
  "fmt"
  "net/http"

  "github.com/husobee/vestigo"
)

func main() {
  router := vestigo.NewRouter()

  // Handle all other requests
  router.Get("/*", ServeIndex)

  // Userlist
  router.Get("/users/", UserIndex)
  router.Get("/users/:name", UserShow)

  // Handle books
  router.Get("/books/", BookIndex)
  router.Get("/books/:book", BookShow)

  // Handle bookcase
  router.Get("/bookcase/", BookcaseIndex)
  router.Get("/bookcase/:book", BookcaseShow)
  router.Get("/bookcase/:book/bookmarks", BookmarksIndex)

  http.ListenAndServe(":8080", router)
}

func FileExists(Filename string) bool {
  return Filename == "./static/css/demo.css" 
}

func ServeIndex(w http.ResponseWriter, r *http.Request) {
  var Filename string

  Filename = vestigo.Param(r, "_name")

  if Filename == "" {
      fmt.Fprintln(w, "Sending index,html")
  } else {
      if FileExists("./static/" + Filename) {
          fmt.Fprintf(w, "Sending %s\n", "./static/" + Filename)
      } else {
          fmt.Fprintf(w, "File %s was not found.\n", "./static/" + Filename)
      }
  }
}

func BookIndex(w http.ResponseWriter, r *http.Request) {
  fmt.Fprintln(w, "Here's our list of books ...")
}

func BookShow(w http.ResponseWriter, r *http.Request) {
  fmt.Fprintf(w, "Showing a book %s\n", vestigo.Param(r, "book"))
}

func BookcaseIndex(w http.ResponseWriter, r *http.Request) {
  fmt.Fprintln(w, "Here's your bookcase")
}

func BookcaseShow(w http.ResponseWriter, r *http.Request) {
  fmt.Fprintf(w, "Here is your book %s\n", vestigo.Param(r, "book"))
}

func BookmarksIndex(w http.ResponseWriter, r *http.Request) {
  fmt.Fprintf(w, "Your bookmarks for your book %s\n", vestigo.Param(r,
"book"))
}

func UserIndex(w http.ResponseWriter, r *http.Request) {
  fmt.Fprintln(w, "All users ...")
}

func UserShow(w http.ResponseWriter, r *http.Request) {
  fmt.Fprintf(w, "You're watching at the profile of user %s\n",
vestigo.Param(r, "name"))
}

Request	Answer	Expected?
/	Sending index,html	Yes
/css/demo.css	Sending ./static/css/demo.css	Yes
/js/demo.js	File ./static/js/demo.js was not found.	Yes
/b	Not Found	No
/books	Showing a book s	No
/books/	Here's our list of books ...	Yes
/books/sonea	Showing a book sonea	Yes
/books/sonea/	Not Found	Yes
/bookcase	Here is your book case	No
/bookcase/	Here's your bookcase	Yes
/bookcase/sonea	Here is your book sonea	Yes
/bookcase/sonea/	Not Found	Yes
/bookcase/sonea/b	Not Found	Yes
/bookcase/sonea/bookmarks	Your bookmarks for your book sonea	Yes

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/husobee/vestigo/issues/49#issuecomment-271731328

[christopher-kleine]

Good morning.

I just check it again on a different PC. Seems like didn't clear all of my cache on the other PC. Here it works like a charm. Well, at least almost. Considering the table above, the following changed:

Request	Answer	Expected? (Expected answer)
/b	File ./static/b was not found.	Yes
/book	Not Found	No (File ./static/book was not found.)
/books	Not Found	No, I guess? (File ./static/books) was not found.
/bookc	Not Found	No (File ./static/bookc was not found.)
/u	File ./static/u was not found.	Yes
/users	File ./static/users was not found.	Yes
/users/	All users ...	Yes
/users/c	You're watching at the profile of user c	Yes

Yesterday, I forgot to include the last 4 cases in the result tabe. Even though I tested them ... Now they work like intended. Great job. Just the other 3(?) cases remaining. Hope that helps a little.

// EDIT: I tried to disable to /users/ route and then call it. It says "Not Found". I guess that's in the "expected" range?

[husobee]

Here is the tree that is generated from your example application:

├── /, 0xc420012780: type=0, parent=0x0,. │ ├── users/, 0xc420012820: type=0, parent=0xc420012780 │ │ └── :, 0xc4200127d0: type=1, parent=0xc420012780 │ ├── book, 0xc420012870: type=0, parent=0xc420012780 │ │ ├── s/, 0xc420012910: type=0, parent=0xc420012870 │ │ │ └── :, 0xc4200128c0: type=1, parent=0xc420012870 │ │ └── case/, 0xc420012960: type=0, parent=0xc420012870 │ │ └── :, 0xc4200129b0: type=1, parent=0xc420012960 │ │ └── /bookmarks, 0xc420012a00: type=0, parent=0xc4200129b0 │ └── *, 0xc420012a50: type=2, parent=0xc420012780

What appears to be happening when we call /book is we actual match a node with no resources attached to it, hence the "Not Found" message from the router, what we should be doing is seeing if this node has no resources, and if it doesn't, try to find an ancestor with a wildcard match. When we call /books, our current node in the search is the s/ node under the book node, and therefore a grand child of the node including the wildcard. The current logic just says, does the parent have a child who is a wildcard, where it should check all ancestors in the chain for a wildcard.

I have updated the branch with a proposed fix.

[christopher-kleine]

Hi,

sorry for my late response to this. Now it works like a charm. I can't get it to fail. No matter what I try. Something I noticed - and I'm not sure whether it's intended or not - is this change:

Let's say we remove the "/users/" route, but keep the "/users/:name" route. Now we request exactly /users/ Before: 404 Now: File ./static/users/ was not found.

This is just something I noticed. Not sure if this is a bug or not. I'm fine with both and I could argue in favor of both cases.

At my workplace we worked with Gin before. Gin was actually quite nice and since we were new to Golang, I didn't have that itch with the context instead of the default handling. As the project progressed, we had to learn that Gin was not the solution. We had to remove it from our entire project. My boss was not pleased that we had to spend a whole week to remove it and to get the app to work again. We then used the default muxer of Golang - until our API became too complex to be easily handled that way. So we used Gorilla Mux. The migration was actually a LOT faster than before. And then ... I found Vestigo. I'm pretty sure my boss will kill me if I tell him, I'd like to switch to a different, better router. But I think that's worth it. :smile: And I'll recommend it to my colleges. And obviously I'll use it myself.

So: Thank you again for this excellent router and also thank you for your hard work. :+1:

// EDIT: I'd say this is a case closed.

[husobee]

Great to hear. I merged that PR, so these changes should be in master now.