echo icon indicating copy to clipboard operation
echo copied to clipboard

question: group middleware adds NotFoundHandler to `prefix` and `prefix*`, but it works without it anyway

Open yudintsevegor opened this issue 2 years ago • 2 comments

Issue Description

Hello team!

I have a question about the implementation detail of the echo. When I try to use an echo.Group with middleware, I found that there were several extra paths with NotFoundHandler handlers that had been registered.

g.Any("", NotFoundHandler)
g.Any("/*", NotFoundHandler)

Why is it implemented like this? I have the assumption that it could be removed. I find really confusing to have these prefix and prefix* when I call Echo.Routes

If we look at the ServeHTTP function, then in the case of an unknown path NotFoundHandler will be executed anyway due to a value in the handler field.

I rectified a little a file group.go locally and unknown paths still return the correct 404.

Checklist

  • [x] Dependencies installed
  • [x] No typos
  • [x] Searched existing issues and docs (I looked to throw the questions https://github.com/labstack/echo/issues?q=is%3Aissue+is%3Aopen+label%3Aquestion)

Expected behavior

routes in the logs if we remove

g.Any("", NotFoundHandler)
g.Any("/*", NotFoundHandler)
2022/08/31 16:07:48 path /path/one
2022/08/31 16:07:48 path /path/two
2022/08/31 16:07:48 path /group/three
⇨ http server started on [::]:9999

Actual behavior

routes in the logs:

2022/08/31 16:06:30 path /group
2022/08/31 16:06:30 path /group
2022/08/31 16:06:30 path /group
2022/08/31 16:06:30 path /group
2022/08/31 16:06:30 path /group/*
2022/08/31 16:06:30 path /group
2022/08/31 16:06:30 path /group
2022/08/31 16:06:30 path /group/*
2022/08/31 16:06:30 path /group/*
2022/08/31 16:06:30 path /group/*
2022/08/31 16:06:30 path /group
2022/08/31 16:06:30 path /group
2022/08/31 16:06:30 path /group/*
2022/08/31 16:06:30 path /group/*
2022/08/31 16:06:30 path /group/*
2022/08/31 16:06:30 path /group/*
2022/08/31 16:06:30 path /group/*
2022/08/31 16:06:30 path /group/three
2022/08/31 16:06:30 path /group
2022/08/31 16:06:30 path /path/two
2022/08/31 16:06:30 path /group
2022/08/31 16:06:30 path /group
2022/08/31 16:06:30 path /group/*
2022/08/31 16:06:30 path /group/*
2022/08/31 16:06:30 path /path/one
⇨ http server started on [::]:9999

Steps to reproduce

With help of the command go mod vendor I changed the group.go file https://github.com/labstack/echo/blob/master/group.go#L28

to

// g.Any("", NotFoundHandler)
// g.Any("/*", NotFoundHandler)

then in the first terminal execute the code below

go run main.go

In the second terminal send the curl requests

# curl localhost:9999/group
{"message":"Not Found"}

curl localhost:9999/group/
{"message":"Not Found"}

curl  localhost:9999/path       
{"message":"Not Found"}

and curl localhost:9999/group/three returns the correct response with 200 OK.

Working code to debug

package main

import (
	"errors"
	"fmt"
	"log"
	"net/http"

	"github.com/labstack/echo/v4"
)

func main() {
	e := echo.New()
	e.HideBanner = true

	mw := func(handlerFunc echo.HandlerFunc) echo.HandlerFunc {
		return handlerFunc
	}

	h := func(c echo.Context) error { return nil }

	e.GET("/path/one", h)
	e.GET("/path/two", h)

	gr := e.Group("/group", mw)
	{
		gr.GET("/three", h)
	}

	for _, r := range e.Routes() {
		log.Println("path", r.Path)
	}

	e.Logger.Fatal(e.Start(":9999"))
}

Version/commit

I tried to play with the version github.com/labstack/echo/v4 v4.8.0

yudintsevegor avatar Aug 31 '22 14:08 yudintsevegor

You might want to read https://github.com/labstack/echo/pull/1728#issuecomment-752231745

TLDR: these are there because group middlewares are executed only when there is a route match and these 2 guarantee a match

aldas avatar Aug 31 '22 18:08 aldas

NB: these notfound routes have been removed from group in v5

aldas avatar Aug 31 '22 18:08 aldas

closing, see https://github.com/labstack/echo/pull/2411 and v5. Current behavior is not going to change in v4.

aldas avatar May 23 '23 21:05 aldas