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

[question] Application failed against latest mux cod base. Any change required? #528

Closed
ccychan opened this issue Oct 30, 2019 · 4 comments
Labels

Comments

@ccychan
Copy link

ccychan commented Oct 30, 2019

Describe the problem you're having

A clear and concise description of what the bug is.

  • Using basic implementation (see below) to register url path and listen to query,
    it's noticed that URL is either not registered properly or not identifiable when complied with
    latest mux code base.
  • Receive "404 page not found" when send http request to application.
  • Verified the same application code works with mux master branch commit tag
    884b5ff.

Are there new changes required to get it to work with the latest?

Versions

Go version: go version
1.12.1

package version: run git rev-parse HEAD inside the repo

"Show me the code!"

A minimal code snippet can be useful, otherwise we're left guessing!

Register url path:
func registerPaths() *mux.Router {
router := mux.NewRouter()
router.Schemes("http")
router.Methods("GET").Path("/api").HandlerFunc(restHandler)
return router
}

Server initialization:
func Init(router *mux.Router) {

// Initialize wait duration
var wait time.Duration

flag.DurationVar(&wait, "graceful-timeout", time.Second * gracefulTimeout,
     "the duration for which the server gracefully wait for existing connections to finish")
flag.Parse()

server := &http.Server{
              Handler:      router,
              Addr:         s.address,
              WriteTimeout: gracefulTimeout * time.Second,
              ReadTimeout:  gracefulTimeout * time.Second,
              IdleTimeout:  idleTimeout * time.Second,
          }

// Server started listen to request
go func() {
  err := server.ListenAndServe()
  if err != nil {
      log.Error(err)
  }
}()

log.Infoln("Started HTTP Server")

}

Hint: wrap it with backticks to format it

@fharding1
Copy link
Contributor

fharding1 commented Oct 30, 2019

This pull request related to schemes was recently merged: #474

You are calling router.Schemes with just http and no handler. This will register a route that only accepts http scheme requests and does nothing with them (404s). I am guessing what you intended to do was:

router.Schemes("http").Methods("GET").Path("/api").HandlerFunc(restHandler)

To further demonstrate why this is happening, take this code for example:

package main

import (
	"net/http"

	"github.com/gorilla/mux"
)

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

	router.Schemes("http").HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
		res.Write([]byte("hi from scheme handler"))
	})

	router.Methods("GET").Path("/api").HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
		res.Write([]byte("hi from other handler"))
	})

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

Since we register the scheme handler first, it is going to match all of our http requests and intercept all of them before we even get to the path handler. If you reverse the order that we register them, you will notice that we match on the path handler and then fall back to the scheme handler on any other http requests.

@ccychan
Copy link
Author

ccychan commented Oct 30, 2019

Hi, thanks for the reply.
Yes, it's true that I want to register multiple hierarchical paths with http as scheme only.
Per what you described, I should do the following

router.Methods("GET").Path("/api").HandlerFunc(restHandler)
router.Methods("GET").Path("/api/a").HandlerFunc(restHandler)
router.Methods("GET").Path("/api/b").HandlerFunc(restHandler)
router.Schemes("http").HandlerFunc(schemesHandler)

if true, does it mean that it will match https request as well?

@fharding1
Copy link
Contributor

If you want to register multiple paths that only match on HTTP, you would first create a scheme matcher route for HTTP, and then register method/path matchers on that route. You could use a sub router, and it might look something like this:

package main

import (
	"net/http"

	"github.com/gorilla/mux"
)

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

	httpOnly := router.Schemes("http").Subrouter()

	httpOnly.Methods("GET").Path("/api").HandlerFunc(func(res http.ResponseWriter, _ *http.Request) { res.Write([]byte("root")) })
	httpOnly.Methods("GET").Path("/api/a").HandlerFunc(func(res http.ResponseWriter, _ *http.Request) { res.Write([]byte("a")) })
	httpOnly.Methods("GET").Path("/api/b").HandlerFunc(func(res http.ResponseWriter, _ *http.Request) { res.Write([]byte("b")) })

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

It will match https if you add https to the list of schemes. Keep in mind that if you don't add a Schemes check at all it will match http and https anyways.

@ccychan
Copy link
Author

ccychan commented Oct 30, 2019

Thanks for the advice. This works out well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants