Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Escaping colon does not work #2046

Closed
nightwolfz opened this issue Dec 16, 2021 · 1 comment · Fixed by #2047
Closed

Escaping colon does not work #2046

nightwolfz opened this issue Dec 16, 2021 · 1 comment · Fixed by #2047

Comments

@nightwolfz
Copy link

nightwolfz commented Dec 16, 2021

Issue Description

The PR from #1988 doesn't seem to work.

Expected behaviour

should match routes

Actual behaviour

none of the routes are matched

Steps to reproduce

Code below

Working code to debug

package main

import (
	"github.com/imroc/req"
	"github.com/labstack/echo/v4"
	"github.com/stretchr/testify/assert"
	"net/http"
	"testing"
	"time"
)

func TestEscape(t *testing.T) {
	e := echo.New()
	e.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
		return func(c echo.Context) error {
			t.Logf("%s %s", c.Request().Method, c.Request().URL.Path)
			return next(c)
		}
	})
	e.GET("/action/ws\\:list", func(c echo.Context) error {
		return c.String(http.StatusOK, "should match")
	})
	e.GET("/action/ws\\:nooo", func(c echo.Context) error {
		return c.String(http.StatusOK, "should not match")
	})
	go e.Start(":8080")

	time.Sleep(time.Second / 2)
	res, err := req.Get("http://localhost:8080/action/ws:list", req.Param{})
	assert.NoError(t, err)
	t.Log(res) // 404 not found
}

Version/commit

4.6.1

@aldas
Copy link
Contributor

aldas commented Dec 16, 2021

Yep, this is not working correctly and tests are wrong.

Problem is that (code below) is not removing \ from path when it continues and that \ ends up in routing tree.

echo/router.go

Lines 101 to 103 in 7bde9ae

if i > 0 && path[i-1] == '\\' {
continue
}

In routes tree we have "/action/ws\:list" but it should be "/action/ws:list"

Probably something like that would fix this issue

			if i > 0 && path[i-1] == '\\' {
				path = path[:i-1] + path[i:]
				i--
				lcpIndex--
				continue
			}

we remove \ from path and move indexes back for current loop.

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

Successfully merging a pull request may close this issue.

2 participants