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

Add wildcard for method argument in Handle ? #29

Closed
Dot-H opened this issue Apr 17, 2020 · 11 comments
Closed

Add wildcard for method argument in Handle ? #29

Dot-H opened this issue Apr 17, 2020 · 11 comments
Labels
enhancement New feature or request

Comments

@Dot-H
Copy link

Dot-H commented Apr 17, 2020

Hi 👋

Proposal

What do you think about adding the possibility to pass a wildcard to the method argument in the Handle function ?

It would be great to permit to just pass all requests, independently from the method, to the same handler.

Use case

I have a service which for a specific Route:

  • Authenticate the request
  • Passthrough the request to an other service

The wildcard on the URL is super handy here because it permits to just write the base URL to the route (ex: /products/{any:*}) and call my handler doing the authentication and then pass-through. Therefore if I add new endpoints in my destination service, I won't need to update this one.

But since there is no equivalent for the methods, if I add endpoints with new methods in my destination service, I need to think to add equivalents in the authentication one. Making the maintainability harder and more error prone.

Thank you for considering this proposal and for this amazing router!

@toby3d
Copy link

toby3d commented Apr 18, 2020

I think it would be nice to add a separate method to add a route that can accept any methods at the specified address, like

r.HandleAny("/route/path", func(ctx *fasthttp.RequestCtx) { ... })

@savsgio
Copy link
Member

savsgio commented Apr 19, 2020

Hi @Dot-H and @toby3d,

I will implement it soon.

Thanks for your advise!

@savsgio savsgio added the enhancement New feature or request label Apr 19, 2020
@savsgio
Copy link
Member

savsgio commented Apr 24, 2020

Hi @Dot-H and @toby3d,

Why do you need to handle any method from one handler?? When I say any method, i mean unknown methods. That could generate unexpected behaviours and maybe sometimes need to check if the http method is valid.

r.HandleAny("/route/path", func(ctx *fasthttp.RequestCtx) { ... })

This route should accept GET, POST, ... , UNICORN, MYCUSTOMETHOD, etc

The following request returns an status code and other things:

curl -X GET http://example.com/route/path --> 200
curl -X POST http://example.com/route/path --> 200
curl -X UNICORN http://example.com/route/path --> This is valid??? Are you prepare to expect that?

Maybe if you need to authenticate the request you could wrap your handlers (like a middleware) and validate.

Or if you want to handle some methods for the same handler you could do:

validMethods := []string{"GET", "POST", "UNICORN"}
handler := func(ctx *fasthttp.RequestCtx) { ... }

for _, method := range validMethods {
   router.Handle(method, "/some/path", handler)
}

I don't sure to add a wildcard method in the router, because like i said, it could generate unexpected behaviours and errors. So you must need to know what HTTP methods are expected, therefore a dirty code.

@toby3d
Copy link

toby3d commented Apr 24, 2020

@savsgio The Server structure in fasthttp package contains the Handler field, which does not validate methods by accepting any requests.

I expect that I will not have to juggle either router.Router{...}.Handler or fasthttp.RequestHandler in fasthttp.Server{ Handler: ... } to accept the prescribed methods in some router.Router routes along with routes where the method is not important to me.

@savsgio
Copy link
Member

savsgio commented Apr 24, 2020

So if if the method it's not important for you, the routes are important or always will you use a wild path (/path/{any:*})??

I ask you, because if it's true. Use fasthttp without the router is the best option. (In your case)

@savsgio
Copy link
Member

savsgio commented Apr 24, 2020

@savsgio The Server structure in fasthttp package contains the Handler field, which does not validate methods by accepting any requests.

I expect that I will not have to juggle either router.Router{...}.Handler or fasthttp.RequestHandler in fasthttp.Server{ Handler: ... } to accept the prescribed methods in some router.Router routes along with routes where the method is not important to me.

It's a good point for add the wild method

@toby3d
Copy link

toby3d commented Apr 24, 2020

So if if the method it's not important for you, the routes are important or always will you use a wild path (/path/{any:*})??

I ask you, because if it's true. Use fasthttp without the router is the best option. (In your case)

For example, I want to use routes for Rest API and for websocket connection:

r := router.New()
r.GET("/users", getAllUsers)
r.GET("/users/{id:[0-9]+}", getSingleUser)
r.POST("/users", createUser)
r.DELETE("/users/{id:[0-9]+}", removeUser)

// NOTE: I don't care about methods, because WebSocket
r.HandleAny("/ws", handleWebSocketActions)
// or, if you just change behaviour for Handle function:
r.Handle("{method:*}", "/ws", handleWebSocketActions)

s := new(fasthttp.Server)
s.Handler = r.Handler

Why WebSocket handler use HandleAny? Because if I use GET route for this, then I always get Switch Protocols in logs.

@savsgio
Copy link
Member

savsgio commented Apr 24, 2020

Why WebSocket handler use HandleAny? Because if I use GET route for this, then I always get Switch Protocols in logs.

And why not? The websocket connection only works if method is GET, if not the returned code should be MethodNotAllowed -> 405

@toby3d
Copy link

toby3d commented Apr 27, 2020

And why not? The websocket connection only works if method is GET, if not the returned code should be MethodNotAllowed -> 405

Suppose I did not choose the most obvious example, but instead of WebSocket, you can take any other HTTP protocol that does not require specific methods or requires a range of them. In any case, the router does not yet support the addition of routes without a rigid binding to methods.

@savsgio
Copy link
Member

savsgio commented May 7, 2020

I'm working on it!

savsgio pushed a commit that referenced this issue May 10, 2020
@savsgio
Copy link
Member

savsgio commented May 10, 2020

Hi @toby3d @Dot-H,

I've just released v1.1.0 😉

Please, check: https://pkg.go.dev/github.com/fasthttp/router?tab=doc#Router.ANY

@savsgio savsgio closed this as completed May 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants