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

Panic when undefined route shares a prefix with a defined route #855

Open
mila-rodriguez-netapp opened this issue Mar 6, 2024 · 4 comments

Comments

@mila-rodriguez-netapp
Copy link

mila-rodriguez-netapp commented Mar 6, 2024

Environment info:

  • KrakenD version: 2.5.0 CE
  • System info: docker / kubernetes
  • Hardware specs:
  • Backend technology: Go
  • Additional environment information:

Describe the bug

My gateway has a number of routes that share a prefix -- eg. /accounts/{id}/widgets, /accounts/{id}/gizmos, etc. These endpoints all route as expected. But if a request is received for a non-existent endpoint that uses the common prefix, this causes krakend to panic.

The panic is handled and logged -- the service does not crash -- but no response is sent to the caller.

Your configuration file:

{
    "$schema": "https://www.krakend.io/schema/v3.json",
    "version": 3,
    "timeout": "30000ms",
    "cache_ttl": "300s",
    "port": 3000,
    "name": "example",
    "disable_keep_alives": true,
    "endpoints": [
      {
        "endpoint": "/accounts/{id}/widgets",
        "method": "GET",
        "output_encoding": "no-op",
        "backend": [{...}]
      },
      {
        "endpoint": "/accounts/{id}/gizmos",
        "method": "GET",
        "output_encoding": "no-op",
        "backend": [{...}]
      },
    ...
  ]
}

The backends really don't matter here, we never get there in this example.

Commands used

krakend run -c /etc/krakend/krakend.json

Test commands:

# test invalid/non-existent route
$ curl -H "Authorization: Bearer <omitted>" http://127.0.0.1:30000/foo
404 page not found

# test valid route
$ curl -H "Authorization: Bearer <omitted>" http://127.0.0.1:30000/accounts/ff424c23-e5f3-47c8-83a6-1914e55b4231/widgets
<success response omitted>

# test invalid/non-existent route with /accounts/{id} prefix
$ curl -H "Authorization: Bearer <omitted>" http://127.0.0.1:30000/accounts/ff424c23-e5f3-47c8-83a6-1914e55b4231/foo
curl: (52) Empty reply from server

Expected behavior

I would expect a query against a undefined endpoint to resolve consistently (502, 404, not particularly choosy as long as it's consistent) and not panic.

In my current version of Krakend, the expected behavior is a 404:

$ curl -H "Authorization: Bearer <omitted>" http://127.0.0.1:30000/foo
404 page not found

instead of:

$ curl -H "Authorization: Bearer <omitted>" http://127.0.0.1:30000/accounts/ff424c23-e5f3-47c8-83a6-1914e55b4231/foo
curl: (52) Empty reply from server

... which is what happens due to the panic.

Logs

2023/10/25 19:01:43 http: panic serving 10.244.17.27:37996: invalid node type
goroutine 8679430 [running]:
net/http.(*conn).serve.func1()
    /usr/local/go/src/net/http/server.go:1854 +0xbf
panic({0x2973800, 0x356a850})
    /usr/local/go/src/runtime/panic.go:890 +0x263
github.com/gin-gonic/gin.(*node).findCaseInsensitivePathRec(0xc00053f2e0?, {0xc000c8441d?, 0x4801b40?}, {0xc000648980?, 0x28?, 0x1?}, {0x2f, 0x0, 0x0, 0x0}, ...)
    /go/pkg/mod/github.com/gin-gonic/[email protected]/tree.go:862 +0xa9d
github.com/gin-gonic/gin.(*node).findCaseInsensitivePathRec(0x8?, {0xc000c84416?, 0xc00053f3c8?}, {0xc000648980?, 0x357d600?, 0xc000ab2000?}, {0x2f, 0x0, 0x0, 0x0}, ...)
    /go/pkg/mod/github.com/gin-gonic/[email protected]/tree.go:778 +0x45d
github.com/gin-gonic/gin.(*node).findCaseInsensitivePathRec(0x0?, {0xc000c84415?, 0x7feb68408188?}, {0xc000648980?, 0xc00053f518?, 0xc00053f560?}, {0x63, 0x0, 0x0, 0x0}, ...)
    /go/pkg/mod/github.com/gin-gonic/[email protected]/tree.go:778 +0x45d
github.com/gin-gonic/gin.(*node).findCaseInsensitivePathRec(0x0?, {0xc000c84414?, 0x0?}, {0xc000648980?, 0xc000b49b23?, 0x3583488?}, {0x61, 0x0, 0x0, 0x0}, ...)
    /go/pkg/mod/github.com/gin-gonic/[email protected]/tree.go:778 +0x45d
github.com/gin-gonic/gin.(*node).findCaseInsensitivePath(0xc000c84414?, {0xc000c84414, 0x43}, 0xd8?)
    /go/pkg/mod/github.com/gin-gonic/[email protected]/tree.go:669 +0x9c
github.com/gin-gonic/gin.redirectFixedPath(0xc000f46500, 0xc000c84414?, 0x8b?)
    /go/pkg/mod/github.com/gin-gonic/[email protected]/gin.go:691 +0x5b
github.com/gin-gonic/gin.(*Engine).handleHTTPRequest(0xc000582b60, 0xc000f46500)
    /go/pkg/mod/github.com/gin-gonic/[email protected]/gin.go:629 +0x3aa
github.com/gin-gonic/gin.(*Engine).ServeHTTP(0xc000582b60, {0x7feb3f599730?, 0xc0011308a0}, 0xc000ced000)

Additional context
I'm actually running a custom plugin so my stack trace might look weird (had to remove the bits specific to my company's application due to policy.) However since #779 looks suspiciously like the same thing I'm seeing, I'm quite certain this hasn't anything to do with any custom code.

Due to the nature of my application, I can't use "disable_redirect_fixed_path": true, which I understand from #779 may be a workaround.

@alombarte
Copy link
Member

alombarte commented Mar 7, 2024

Hello @mila-rodriguez-netapp ,

Have you been able to reproduce this behavior in a clean environment without plugins? I just copy/pasted your example, added a default backend and I was unable to reproduce this behavior. Could you provide more input? The issue you refer to, does not panic in 2.5

{
    "$schema": "https://www.krakend.io/schema/v3.json",
    "version": 3,
    "timeout": "30000ms",
    "cache_ttl": "300s",
    "port": 8080,
    "name": "polariscloud",
    "disable_keep_alives": true,
    "host": ["http://localhost:8080"],
    "debug_endpoint": true,
    "endpoints": [
      {
        "endpoint": "/accounts/{id}/widgets",
        "method": "GET",
        "output_encoding": "no-op",
        "backend": [{"url_pattern": "/__debug/gizmos"}]
      },
      {
        "endpoint": "/accounts/{id}/gizmos",
        "method": "GET",
        "output_encoding": "no-op",
        "backend": [{"url_pattern": "/__debug/gizmos"}]
      }
  ]
}

Curls:

 ~ ❯ curl http://localhost:8080/accounts/12/gizmos
{"message":"pong"}

~ ❯ curl http://localhost:8080/accounts/12/giszmos
404 page not found

~ ❯ curl http://localhost:8080/accounts/12/widgets
{"message":"pong"}

~ ❯ curl http://localhost:8080/accounts/1dasdasd2/widgets
{"message":"pong"}

~ ❯ curl http://localhost:8080/accountsa/1dasdasd2/widgets
404 page not found

~ ❯ curl http://localhost:8080/accounts/1234/widgets
{"message":"pong"}

~ ❯ curl http://localhost:8080/accounts/1234/widgetsaaa
404 page not found

@MichelFortes
Copy link

Same here, my friends.
Here is the conf file. It's kind of large...

curl -i [your-host]/group/v2/units

krakend_config.json

@alombarte
Copy link
Member

Hey, you are facing a problem as described in the issues #779, #325, #386, #423 and #632 related to routing conflicts leading to errors when using automated processes to generate configuration files with multiple endpoints.

This is indeed a known issue with the router package used internally by KrakenD. We have communicated with the maintainers of the router package about this problem twice (gin-gonic/gin#2959). Initially, there was an attempt to fix it, but unfortunately, that effort did not fully resolve the issue. We are waiting for the package maintainers' response to find a permanent solution.

In the meantime, the workaround you've discovered —disabling the automatic redirect— remains the only method to prevent these panics. This situation occurs when a 404 response is expected for a request to a path that overlaps with several patterns but matches none of them.

@kpacha
Copy link
Member

kpacha commented May 16, 2024

I've created a new issue in the gin project (gin-gonic/gin#3972), hoping to get some attention because it looks like the old one is getting none since it's already closed and marked as fixed

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

No branches or pull requests

4 participants