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

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.

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

Successfully merging a pull request may close this issue.

2 participants