value.tsr cause wrong redirect

[Zeahow]

Version: v1.7.4

After upgrading the version from v1.7.2 to v1.7.4, the logic of value.tsr has changed. Some cases that should return false will return true now.

I register a handler with path logout, while path login is handled in NoRouter handler. Only login will be handled in NoRouter. NoRouter will return 404 if path is login/.

In v1.7.2 login will enter NoRouter, and it will be handled. But in v1.7.4, value.tsr will be true so that the request will be rediredcted to login/ which will return 404.

[qm012]

Could you provide an example?
In some cases panic can occur

prefix[len(path)] // panic  

[Zeahow]

Could you provide an example?
In some cases panic can occur

prefix[len(path)] // panic  

It won't panic.

The code in screenshot is to determine whether a leaf exists for the same URL with an external trailing slash. For example, for path login, it will got true, if there exists a leaf with path login/. But in v1.7.4, it will got true too, if there exists a logout leaf , and even if there not exists a login/ leaf. It will got true as long as there exists any leaf with a 6-char(6=len("login"+1)) path.

[qm012]

Could you provide an example?

Something like this

package main

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

	"github.com/gin-gonic/gin"
)

func main() {
	router := gin.New()
	fmt.Println(gin.Version)

	server := &http.Server{
		Addr:    ":8080",
		Handler: router,
	}

	router.GET("/", func(c *gin.Context) {
		c.String(200, c.ClientIP())
	})

	err := server.ListenAndServe()
	if err != nil {
		log.Fatal(err)
	}
}

When I changed the code, I removed it because there was a panic. But I did not record the problem of panic. -_ -

[Zeahow]

Could you provide an example?

Something like this

package main

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

	"github.com/gin-gonic/gin"
)

func main() {
	router := gin.New()
	fmt.Println(gin.Version)

	server := &http.Server{
		Addr:    ":8080",
		Handler: router,
	}

	router.GET("/", func(c *gin.Context) {
		c.String(200, c.ClientIP())
	})

	err := server.ListenAndServe()
	if err != nil {
		log.Fatal(err)
	}
}

When I changed the code, I removed it because there was a panic. But I did not record the problem of panic. -_ -

package main

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

	"github.com/gin-gonic/gin"
)

func main() {
	router := gin.New()
	fmt.Println(gin.Version)

	server := &http.Server{
		Addr:    ":8080",
		Handler: router,
	}

	router.NoRoute(func(c *gin.Context) {
		if c.Request.RequestURI == "/login" {
			c.String(200, "Enter NoRoute. Path: login")
		}
	})

	router.GET("/logout", func(c *gin.Context) {
		c.String(200, "logout")
	})

	err := server.ListenAndServe()
	if err != nil {
		log.Fatal(err)
	}
}

Visit localhost:8080/login in brower:
v1.7.4: 404 page not found
v1.7.2: Enter NoRoute. Path: login

Tip: Clean brower cache or open a private windows, after switching v1.7.4 to v1.7.2

In addition, I could not understand how prefix[len(path)] panic. Its previous condition len(prefix) == len(path)+1 promises that prefix is longger than path.

[qm012]

Ok, thank you for the example you provided. I am a little busy these two days, so I will test it on Sunday 💗

[luchuanbing123]

Agree with @Zeahow , path == prefix[:len(prefix)-1] can't be deleted.
Otherwise, /login will be redirct to /login/(not exsit), because len("/logout") == len("/login") +1

[qm012]

@Zeahow Yes, there will be a suffix '/' at the end of the match, can you submit a PR to fix this problem? And add test example, thanks.

[citizen233]

@appleboy @qm012 I fixed this question and submitted pr

[appleboy]

fixed in #2847