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

Not able to detect the route url without \ #2547

Open
PuneetAgrawal-plivo opened this issue Nov 16, 2023 · 9 comments
Open

Not able to detect the route url without \ #2547

PuneetAgrawal-plivo opened this issue Nov 16, 2023 · 9 comments
Assignees

Comments

@PuneetAgrawal-plivo
Copy link

Issue Description

I had created routes using POST with path parameters. If the value is not passed in path params it is not able to read the route and if the value is passed it is able to read it

route("VerifiedCallerId/Verification/:verification_uuid", tc.ValidateOtp, POST),

if the :verification_uuid is not passed then getting 405 if the value is passed working properly

Checklist

  • [ yes] Dependencies installed
  • [ yes] No typos
  • [ yes] Searched existing issues and docs

Expected behaviour

it should go to the path function and process

Actual behaviour

it is returning method not allowed
Status code : 405
{
"message": "Method Not Allowed"
}

Steps to reproduce

Working code to debug

package main

func main() {
}

Version/commit

@aldas
Copy link
Contributor

aldas commented Nov 16, 2023

Please check your server logs if that request is actually sent as POST? Status code 405 hints that OPTIONS method is being used.

or maybe you could create working example. You can start from this:

func main() {
	e := echo.New()
	e.Use(middleware.Logger())

	e.POST("VerifiedCallerId/Verification/:verification_uuid", func(c echo.Context) error {
		return c.String(http.StatusOK, "OK: "+c.Param("verification_uuid"))
	})

	if err := e.Start(":8080"); err != nil && !errors.Is(err, http.ErrServerClosed) {
		e.Logger.Fatal(err)
	}
}
curl -v -d "param1=value1&param2=value2" -X POST http://localhost:8080/VerifiedCallerId/Verification/my_uid

@PuneetAgrawal-plivo
Copy link
Author

I have checked the method is post.
There is one more method below this which
route("VerifiedCallerId/:phone_number", tc.GetVerifiedID, GET),

it is picking this route as Verification as :phone_number and then returning method not allowed.

Ideally it should pick the above route because of longest matching path but it is picking this one

@aldas
Copy link
Contributor

aldas commented Nov 16, 2023

Please try to recreate that situation with that example and add appropriate curl so that I can test this out.

@PuneetAgrawal-plivo
Copy link
Author

PuneetAgrawal-plivo commented Nov 16, 2023

func main() {
	e := echo.New()
	e.Use(middleware.Logger())

	e.POST("VerifiedCallerId/Verification/:verification_uuid", func(c echo.Context) error {
		return c.String(http.StatusOK, "OK")
	})
	e.GET("VerifiedCallerId/:phone_number", func(c echo.Context) error {
		return c.JSONPretty(http.StatusOK, map[string]interface{}{"message": "welcome to Plivo"}, " ")
	})

	if err := e.Start(":5000"); err != nil && !errors.Is(err, http.ErrServerClosed) {
		e.Logger.Fatal(err)
	}
}

You cun run this file go run recreate.go

and then hit this curl
curl -v --request POST 'http://localhost:5000/VerifiedCallerId/Verification/'

you can reproduce the same

@aldas
Copy link
Contributor

aldas commented Nov 16, 2023

ok, I see. Currently then the path param is last segment in path it does not match empty values. This is why you get 405 as it matches path for that GET route but method is different.

Currently workaround for this situation is to use * any route and get that value param value

	e.POST("VerifiedCallerId/Verification/*", func(c echo.Context) error {
		return c.String(http.StatusOK, "OK:"+c.Param("*"))
	})

@aldas
Copy link
Contributor

aldas commented Nov 16, 2023

note to self: this line is probably the reason why empty path params at the end are not matched

echo/router.go

Line 654 in 584cb85

if child := currentNode.paramChild; search != "" && child != nil {

and same reason why empty any segment works is here

echo/router.go

Line 676 in 584cb85

if child := currentNode.anyChild; child != nil {

I need to understand why we are doing this and then we can decide if this can be changed and asses the implication of that change

@PuneetAgrawal-plivo
Copy link
Author

Thanks for the info

@ProgrammingMuffin
Copy link

@aldas how should the processing proceed in this scenario. Should it throw a 404 or should it let the endpoint process it if there is a "/"?

@aldas aldas self-assigned this Mar 13, 2024
@aldas
Copy link
Contributor

aldas commented Apr 6, 2024

One workaround would be to change from e.POST("a/b/:uid", h) to e.POST("a/b/*", h) and access c.Param("uid") with c.Param("*").

Path params (:uid) and match-any (*) work little bit differently when comes to the end of request url.
I remember that path params allow empty values when it is in the middle of route path. For example Route with path e.GET("v1/:1/list/:2", h) matches with request /v1//list/test

note:

this may not be that simple. Consider this case

a)

	e.GET("/*", handlerFunc) // 1
	e.GET("/users/", handlerFunc) // 2
	e.GET("/users/:id", handlerFunc) // 3

b)

	e.GET("/*", handlerFunc) // 1
	e.GET("/users/:id", handlerFunc) // 2

when the request URL is /users/ what should be the correct match for a) and b)

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

No branches or pull requests

3 participants