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

With RedirectFixedPath enabled, the request execution panics when a 404 is expected #2959

Closed
kpacha opened this issue Nov 25, 2021 · 4 comments · Fixed by #3227
Closed

With RedirectFixedPath enabled, the request execution panics when a 404 is expected #2959

kpacha opened this issue Nov 25, 2021 · 4 comments · Fixed by #3227
Labels

Comments

@kpacha
Copy link

kpacha commented Nov 25, 2021

Description

When a request to a path doesn't match any registered patterns and the RedirectFixedPath flag is set to true, the request execution panics (invalid node type). This is the line throwing the panic: https://github.com/gin-gonic/gin/blob/v1.7.7/tree.go#L851

If the panic is commented, everything seems to work as expected with the exception of the test TestTreeInvalidNodeType.

How to reproduce

Add this function to the gin_integration_test.go file

func TestRedirectFixedPath(t *testing.T) {
	router := Default()

	router.RedirectFixedPath = true

	router.GET("/aa/aa", func(c *Context) { c.String(http.StatusOK, "/aa/aa") })
	router.GET("/:bb/aa", func(c *Context) { c.String(http.StatusOK, "/:bb/aa") })

	ts := httptest.NewServer(router)
	defer ts.Close()

	testRequest(t, ts.URL+"/aa/aa", "", "/aa/aa")
	testRequest(t, ts.URL+"/bb/aa", "", "/:bb/aa")

	testRequest(t, ts.URL+"/aa", "404 Not Found") // this request makes the router panic
}

Expectations

The router should return a 404

Actual result

The router panics and the client (curl) returns

curl: (52) Empty reply from server

Environment

  • go version: 1.17.3
  • gin version (or commit ref): 1.7.7
  • operating system: linux
@LawyZheng
Copy link

LawyZheng commented Jan 10, 2022

Also the Same Panic For Case-Insensitive Redirect.
I haven't tried it after commenting out the panic yet.
So I'm not sure it is a bug or not.

testRequest(t, ts.URL+"/aa/AA", "/aa/aa")  // this request also makes the router panic

This test case should be returned as /aa/aa, if I have understood the following explanation correctly.

gin.go:line 74	// For example /FOO and /..//Foo could be redirected to /foo.

Environment

  • go version: 1.17.3
  • gin version: 1.7.4
  • os: windows

@kpacha
Copy link
Author

kpacha commented Mar 21, 2023

@appleboy , it looks like this issue is still happening on v1.9.0. You just need to add this test to reproduce (as I suggested on the bug report).

func TestRedirectFixedPath(t *testing.T) {
	router := gin.Default()

	router.RedirectFixedPath = true

	router.GET("/aa/aa", func(c *gin.Context) { c.String(http.StatusOK, "/aa/aa") })
	router.GET("/:bb/aa", func(c *gin.Context) { c.String(http.StatusOK, "/:bb/aa") })

	ts := httptest.NewServer(router)
	defer ts.Close()

	testRequest(t, ts.URL+"/aa/aa", "", "/aa/aa")
	testRequest(t, ts.URL+"/bb/aa", "", "/:bb/aa")

	// the following requests panic if the RedirectFixedPath flag is enabled
	testRequest(t, ts.URL+"/aa", "404 Not Found")
	testRequest(t, ts.URL+"/aa/aa/aa/aa", "404 Not Found")
	testRequest(t, ts.URL+"/aa/AA", "/aa/aa")
}

this is the result of that test

$ go test -run TestRedirectFixedPath .
[GIN] 2023/03/21 - 17:19:12 | 200 |        7.62µs |       127.0.0.1 | GET      "/aa/aa"
[GIN] 2023/03/21 - 17:19:12 | 200 |       33.66µs |       127.0.0.1 | GET      "/bb/aa"
2023/03/21 17:19:12 http: panic serving 127.0.0.1:59078: invalid node type
goroutine 81 [running]:
net/http.(*conn).serve.func1()
	/usr/lib/go-1.20/src/net/http/server.go:1854 +0xbf
panic({0xac0620, 0xca9de0})
	/usr/lib/go-1.20/src/runtime/panic.go:890 +0x263
github.com/gin-gonic/gin.(*node).findCaseInsensitivePathRec(0x0?, {0xc00002e0b4?, 0x0?}, {0xc0000c2180?, 0xc000416c39?, 0x1?}, {0x0, 0x0, 0x0, 0x0}, ...)
	/<redacted>/gin/tree.go:862 +0xa9d
github.com/gin-gonic/gin.(*node).findCaseInsensitivePath(0xc00002e0b4?, {0xc00002e0b4, 0x3}, 0x18?)
	/<redacted>/gin/tree.go:669 +0x9c
github.com/gin-gonic/gin.redirectFixedPath(0xc0005a4000, 0xc00002e0b4?, 0x58?)
	/<redacted>/gin/gin.go:691 +0x5b
github.com/gin-gonic/gin.(*Engine).handleHTTPRequest(0xc00045a000, 0xc0005a4000)
	/<redacted>/gin/gin.go:629 +0x3aa
github.com/gin-gonic/gin.(*Engine).ServeHTTP(0xc00045a000, {0xcb1650?, 0xc00014c0e0}, 0xc0001e6000)
	/<redacted>/gin/gin.go:576 +0x1dd
net/http.serverHandler.ServeHTTP({0xc000626060?}, {0xcb1650, 0xc00014c0e0}, 0xc0001e6000)
	/usr/lib/go-1.20/src/net/http/server.go:2936 +0x316
net/http.(*conn).serve(0xc0001203f0, {0xcb1fc0, 0xc0001100c0})
	/usr/lib/go-1.20/src/net/http/server.go:1995 +0x612
created by net/http.(*Server).Serve
	/usr/lib/go-1.20/src/net/http/server.go:3089 +0x5ed
--- FAIL: TestRedirectFixedPath (0.00s)
    gin_integration_test.go:43: 
        	Error Trace:	/<redacted>/gin/gin_integration_test.go:43
        	            				/<redacted>/gin/gin_integration_test.go:576
        	Error:      	Received unexpected error:
        	            	Get "http://127.0.0.1:40013/aa": EOF
        	Test:       	TestRedirectFixedPath
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x40 pc=0xa37402]

goroutine 6 [running]:
testing.tRunner.func1.2({0xaedf20, 0x10f4b00})
	/usr/lib/go-1.20/src/testing/testing.go:1526 +0x24e
testing.tRunner.func1()
	/usr/lib/go-1.20/src/testing/testing.go:1529 +0x39f
panic({0xaedf20, 0x10f4b00})
	/usr/lib/go-1.20/src/runtime/panic.go:884 +0x213
github.com/gin-gonic/gin.testRequest(0xc0000e5ba0, {0xc0000ddf08, 0x2, 0xb97d4c?})
	/<redacted>/gin/gin_integration_test.go:44 +0x182
github.com/gin-gonic/gin.TestRedirectFixedPath(0x0?)
	/<redacted>/gin/gin_integration_test.go:576 +0x28f
testing.tRunner(0xc0000e5ba0, 0xbf20c8)
	/usr/lib/go-1.20/src/testing/testing.go:1576 +0x10b
created by testing.(*T).Run
	/usr/lib/go-1.20/src/testing/testing.go:1629 +0x3ea
FAIL	github.com/gin-gonic/gin	0.011s
FAIL

Will you reopen this or should I create a new one?

@pebo
Copy link

pebo commented May 23, 2023

krakend uses gin-gonic (v1.9.0?) and logs these errors:

2023/05/23 14:42:00 http: panic serving 169.254.1.1:60572: invalid node type
goroutine 362961 [running]:
net/http.(*conn).serve.func1()
	/usr/local/go/src/net/http/server.go:1854 +0xbf
panic({0x291e7e0, 0x35156d0})
	/usr/local/go/src/runtime/panic.go:890 +0x263
github.com/gin-gonic/gin.(*node).findCaseInsensitivePathRec(0xc005783640?, {0xc00b799bde?, 0xc005783680?}, {0xc000a57680?, 0x10169d3?, 0xf06f65?}, {0x31, 0x0, 0x0, 0x0}, ...)
	/go/pkg/mod/github.com/gin-gonic/gin@v1.9.0/tree.go:862 +0xa9d
github.com/gin-gonic/gin.(*node).findCaseInsensitivePathRec(0x1280f45?, {0xc00b799bdd?, 0xc001f36120?}, {0xc000a57680?, 0x7?, 0xc005783830?}, {0x31, 0x0, 0x0, 0x0}, ...)
	/go/pkg/mod/github.com/gin-gonic/gin@v1.9.0/tree.go:778 +0x45d
github.com/gin-gonic/gin.(*node).findCaseInsensitivePathRec(0x0?, {0xc00b799bdc?, 0x0?}, {0xc000a57680?, 0xc000930010?, 0xf087d0?}, {0x76, 0x0, 0x0, 0x0}, ...)
	/go/pkg/mod/github.com/gin-gonic/gin@v1.9.0/tree.go:778 +0x45d
github.com/gin-gonic/gin.(*node).findCaseInsensitivePath(0xc00b799bdc?, {0xc00b799bdc, 0xb}, 0x70?)
	/go/pkg/mod/github.com/gin-gonic/gin@v1.9.0/tree.go:669 +0x9c
github.com/gin-gonic/gin.redirectFixedPath(0xc008fc5b00, 0xc00b799bdc?, 0x0?)
	/go/pkg/mod/github.com/gin-gonic/gin@v1.9.0/gin.go:691 +0x5b
github.com/gin-gonic/gin.(*Engine).handleHTTPRequest(0xc000a5cd00, 0xc008fc5b00)
	/go/pkg/mod/github.com/gin-gonic/gin@v1.9.0/gin.go:629 +0x3aa
github.com/gin-gonic/gin.(*Engine).ServeHTTP(0xc000a5cd00, {0x3539350?, 0xc00a47fb20}, 0xc00aa3c100)
	/go/pkg/mod/github.com/gin-gonic/gin@v1.9.0/gin.go:576 +0x1dd
github.com/rs/cors.(*Cors).Handler.func1({0x3539350, 0xc00a47fb20}, 0xc00aa3c100)
	/go/pkg/mod/github.com/rs/cors@v1.8.2/cors.go:231 +0x1c4
net/http.HandlerFunc.ServeHTTP(0x0?, {0x3539350?, 0xc00a47fb20?}, 0xf60aee?)
	/usr/local/go/src/net/http/server.go:2122 +0x2f
net/http.serverHandler.ServeHTTP({0xc001f36090?}, {0x3539350, 0xc00a47fb20}, 0xc00aa3c100)
	/usr/local/go/src/net/http/server.go:2936 +0x316
net/http.(*conn).serve(0xc00b530ab0, {0x353aa78, 0xc007899e60})
	/usr/local/go/src/net/http/server.go:1995 +0x612
created by net/http.(*Server).Serve
	/usr/local/go/src/net/http/server.go:3089 +0x5ed

Could these errors be related to this issue?

@kpacha
Copy link
Author

kpacha commented Jun 2, 2023

maybe I mentioned the wrong person, in that case I'll try again because there has been a new release and this was not discussed:

@thinkerou which one do you think is the best way to proceed? should I open a new issue or will you reopen this one?

I'm sorry if I mentioned the wrong person again. I just want to be sure this is reaching the right people

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

Successfully merging a pull request may close this issue.

4 participants