Repository: gin-gonic/gin
Pull request #2857 (2021-09-06 08:49:37)

feat(router): add literal colon support (#1432)
Implement literal colon support using the approach described in #1432

codecov[bot]: # [Codecov](https://codecov.io/gh/gin-gonic/gin/pull/2857?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) Report
> Merging [#2857](https://codecov.io/gh/gin-gonic/gin/pull/2857?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) (2e6a3ff) into [master](https://codecov.io/gh/gin-gonic/gin/commit/a550c568d7416df89c6b29615a6f977880c668f2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) (a550c56) will **decrease** coverage by `0.35%`.
> The diff coverage is `76.92%`.

[![Impacted file tree graph](https://codecov.io/gh/gin-gonic/gin/pull/2857/graphs/tree.svg?width=650&height=150&src=pr&token=Qpqxhwery8&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic)](https://codecov.io/gh/gin-gonic/gin/pull/2857?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic)

```diff
@@            Coverage Diff             @@
##           master    #2857      +/-   ##
==========================================
- Coverage   98.75%   98.39%   -0.36%     
==========================================
  Files          41       40       -1     
  Lines        3054     3057       +3     
==========================================
- Hits         3016     3008       -8     
- Misses         26       35       +9     
- Partials       12       14       +2     
```

| Flag | Coverage Δ | |
|---|---|---|
| go-1.13 | `?` | |
| go-1.14 | `98.39% <76.92%> (-0.20%)` | :arrow_down: |
| go-1.15 | `?` | |
| go-1.16 | `?` | |
| go-1.17 | `?` | |
| macos-latest | `?` | |
| nomsgpack | `?` | |
| ubuntu-latest | `98.39% <76.92%> (-0.36%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/gin-gonic/gin/pull/2857?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) | Coverage Δ | |
|---|---|---|
| [gin.go](https://codecov.io/gh/gin-gonic/gin/pull/2857/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic#diff-Z2luLmdv) | `97.98% <69.23%> (-1.12%)` | :arrow_down: |
| [tree.go](https://codecov.io/gh/gin-gonic/gin/pull/2857/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic#diff-dHJlZS5nbw==) | `99.64% <84.61%> (-0.36%)` | :arrow_down: |
| [debug.go](https://codecov.io/gh/gin-gonic/gin/pull/2857/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic#diff-ZGVidWcuZ28=) | `91.66% <0.00%> (-8.34%)` | :arrow_down: |
| [binding/binding\_nomsgpack.go](https://codecov.io/gh/gin-gonic/gin/pull/2857/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic#diff-YmluZGluZy9iaW5kaW5nX25vbXNncGFjay5nbw==) | | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/gin-gonic/gin/pull/2857?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/gin-gonic/gin/pull/2857?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic). Last update [a550c56...2e6a3ff](https://codecov.io/gh/gin-gonic/gin/pull/2857?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic).

wssccc: > Still need to optimize and adjust, why define some routing rules like \?

The feature included in this PR is not about defining rule like `\`, but using `\` to escape a colon

Please refer to issue #1432 for more details
wssccc: @cikay @daheige conflicts and issues mentioned above have been resolved
mytram: Need this for my project which follows google's custom method conventions, i.e. `:verb` I am wondering if @cikay @daheige can review it and give it a tick. That will be awesome. Many thanks
mytram: @daheige Thanks a lot 👍 !

mytram: Hi @thinkerou wondering if you can have a look and approve the PR for the next release. Many thanks 🙏 
grindlife: Is the pull request still awaiting review?

I apologize for my poor English.
slnc: @cikay @daheige is anything blocking this PR? I'm asking in case the community can help. This PR would fix an issue my team is having.

Thank you
appleboy: I will take it asap.
appleboy: @wssccc Can you also add http-router benchmark report since you update the router tree structure?
wssccc: > @wssccc Can you also add http-router benchmark report since you update the router tree structure?

done
appleboy: @wssccc I mean the different benchmarks between the master and this PR branch.
appleboy: See the sample: https://github.com/gin-gonic/gin/pull/1826#issuecomment-492477487
wssccc: The pull request benchmark data:
```
@wssccc ➜ /workspaces/go-http-routing-benchmark (master) $ go test -bench=Gin
#GithubAPI Routes: 203
   Gin: 58792 Bytes

#GPlusAPI Routes: 13
   Gin: 4544 Bytes

#ParseAPI Routes: 26
   Gin: 7864 Bytes

#Static Routes: 157
   Gin: 34344 Bytes

goos: linux
goarch: amd64
pkg: github.com/julienschmidt/go-http-routing-benchmark
cpu: AMD EPYC 7763 64-Core Processor                
BenchmarkGin_Param              23936918                50.54 ns/op            0 B/op          0 allocs/op
BenchmarkGin_Param5             12072578               111.9 ns/op             0 B/op          0 allocs/op
BenchmarkGin_Param20             4816608               266.8 ns/op             0 B/op          0 allocs/op
BenchmarkGin_ParamWrite         19148685                64.69 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GithubStatic       19514295                65.10 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GithubParam         8956462               120.7 ns/op             0 B/op          0 allocs/op
BenchmarkGin_GithubAll             57808             19567 ns/op               0 B/op          0 allocs/op
BenchmarkGin_GPlusStatic        26049177                56.40 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GPlusParam         15572720                84.69 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GPlus2Params       11228659               100.1 ns/op             0 B/op          0 allocs/op
BenchmarkGin_GPlusAll            1303134               993.8 ns/op             0 B/op          0 allocs/op
BenchmarkGin_ParseStatic        26377340                48.55 ns/op            0 B/op          0 allocs/op
BenchmarkGin_ParseParam         18363444                60.13 ns/op            0 B/op          0 allocs/op
BenchmarkGin_Parse2Params       11881887                90.09 ns/op            0 B/op          0 allocs/op
BenchmarkGin_ParseAll             730262              1639 ns/op               0 B/op          0 allocs/op
BenchmarkGin_StaticAll             94722             14625 ns/op               0 B/op          0 allocs/op
PASS
ok      github.com/julienschmidt/go-http-routing-benchmark      27.432s
```
The master branch benchmark data:
```
@wssccc ➜ /workspaces/go-http-routing-benchmark (master) $ go test -bench=Gin
#GithubAPI Routes: 203
   Gin: 58792 Bytes

#GPlusAPI Routes: 13
   Gin: 4544 Bytes

#ParseAPI Routes: 26
   Gin: 7864 Bytes

#Static Routes: 157
   Gin: 34344 Bytes

goos: linux
goarch: amd64
pkg: github.com/julienschmidt/go-http-routing-benchmark
cpu: AMD EPYC 7763 64-Core Processor                
BenchmarkGin_Param              23772192                53.62 ns/op            0 B/op          0 allocs/op
BenchmarkGin_Param5             10336761               102.5 ns/op             0 B/op          0 allocs/op
BenchmarkGin_Param20             4763977               271.9 ns/op             0 B/op          0 allocs/op
BenchmarkGin_ParamWrite         15457245                73.56 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GithubStatic       19980954                66.93 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GithubParam        10229772               127.1 ns/op             0 B/op          0 allocs/op
BenchmarkGin_GithubAll             61861             20814 ns/op               0 B/op          0 allocs/op
BenchmarkGin_GPlusStatic        24932599                52.98 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GPlusParam         16225578                80.40 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GPlus2Params       12601467                99.77 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GPlusAll            1204710              1090 ns/op               0 B/op          0 allocs/op
BenchmarkGin_ParseStatic        26572233                51.08 ns/op            0 B/op          0 allocs/op
BenchmarkGin_ParseParam         21089107                62.14 ns/op            0 B/op          0 allocs/op
BenchmarkGin_Parse2Params       16765136                73.76 ns/op            0 B/op          0 allocs/op
BenchmarkGin_ParseAll             737712              1910 ns/op               0 B/op          0 allocs/op
BenchmarkGin_StaticAll             85904             13133 ns/op               0 B/op          0 allocs/op
PASS
ok      github.com/julienschmidt/go-http-routing-benchmark      25.381s
```
wssccc: @appleboy done

Patch:
diff --git a/gin.go b/gin.go
index 57f8c2a..5ba1cf6 100644
--- a/gin.go
+++ b/gin.go
@@ -24,6 +24,9 @@ import (
 )
 
 const defaultMultipartMemory = 32 << 20 // 32 MB
+const escapedColon = "\\:"
+const colon = ":"
+const backslash = "\\"
 
 var (
 	default404Body = []byte("404 page not found")
@@ -474,6 +477,26 @@ func (engine *Engine) validateHeader(header string) (clientIP string, valid bool
 	return "", false
 }
 
+// updateRouteTree do update to the route tree recursively
+func updateRouteTree(n *node) {
+	n.path = strings.ReplaceAll(n.path, escapedColon, colon)
+	n.fullPath = strings.ReplaceAll(n.fullPath, escapedColon, colon)
+	n.indices = strings.ReplaceAll(n.indices, backslash, colon)
+	if n.children == nil {
+		return
+	}
+	for _, child := range n.children {
+		updateRouteTree(child)
+	}
+}
+
+// updateRouteTrees do update to the route trees
+func (engine *Engine) updateRouteTrees() {
+	for _, tree := range engine.trees {
+		updateRouteTree(tree.root)
+	}
+}
+
 // parseIP parse a string representation of an IP and returns a net.IP with the
 // minimum byte representation or nil if input is invalid.
 func parseIP(ip string) net.IP {
@@ -498,7 +521,7 @@ func (engine *Engine) Run(addr ...string) (err error) {
 		debugPrint("[WARNING] You trusted all proxies, this is NOT safe. We recommend you to set a value.\n" +
 			"Please check https://github.com/gin-gonic/gin/blob/master/docs/doc.md#dont-trust-all-proxies for details.")
 	}
-
+	engine.updateRouteTrees()
 	address := resolveAddress(addr)
 	debugPrint("Listening and serving HTTP on %s\n", address)
 	err = http.ListenAndServe(address, engine.Handler())
diff --git a/tree.go b/tree.go
index ce0f065..b0a5f98 100644
--- a/tree.go
+++ b/tree.go
@@ -262,7 +262,19 @@ walk:
 // Returns -1 as index, if no wildcard was found.
 func findWildcard(path string) (wildcard string, i int, valid bool) {
 	// Find start
+	escapeColon := false
 	for start, c := range []byte(path) {
+		if escapeColon {
+			escapeColon = false
+			if c == ':' {
+				continue
+			}
+			panic("invalid escape string in path '" + path + "'")
+		}
+		if c == '\\' {
+			escapeColon = true
+			continue
+		}
 		// A wildcard starts with ':' (param) or '*' (catch-all)
 		if c != ':' && c != '*' {
 			continue


Test patch:
diff --git a/gin_integration_test.go b/gin_integration_test.go
index 2125df9..5398271 100644
--- a/gin_integration_test.go
+++ b/gin_integration_test.go
@@ -577,3 +577,28 @@ func TestTreeRunDynamicRouting(t *testing.T) {
 func isWindows() bool {
 	return runtime.GOOS == "windows"
 }
+
+func TestEscapedColon(t *testing.T) {
+	router := New()
+	f := func(u string) {
+		router.GET(u, func(c *Context) { c.String(http.StatusOK, u) })
+	}
+	f("/r/r\\:r")
+	f("/r/r:r")
+	f("/r/r/:r")
+	f("/r/r/\\:r")
+	f("/r/r/r\\:r")
+	assert.Panics(t, func() {
+		f("\\foo:")
+	})
+
+	router.updateRouteTrees()
+	ts := httptest.NewServer(router)
+	defer ts.Close()
+
+	testRequest(t, ts.URL+"/r/r123", "", "/r/r:r")
+	testRequest(t, ts.URL+"/r/r:r", "", "/r/r\\:r")
+	testRequest(t, ts.URL+"/r/r/r123", "", "/r/r/:r")
+	testRequest(t, ts.URL+"/r/r/:r", "", "/r/r/\\:r")
+	testRequest(t, ts.URL+"/r/r/r:r", "", "/r/r/r\\:r")
+}
diff --git a/tree_test.go b/tree_test.go
index c9b0313..3aa3a59 100644
--- a/tree_test.go
+++ b/tree_test.go
@@ -192,6 +192,7 @@ func TestTreeWildcard(t *testing.T) {
 		"/get/abc/123abg/:param",
 		"/get/abc/123abf/:param",
 		"/get/abc/123abfff/:param",
+		"/get/abc/escaped_colon/test\\:param",
 	}
 	for _, route := range routes {
 		tree.addRoute(route, fakeHandler(route))
@@ -315,6 +316,7 @@ func TestTreeWildcard(t *testing.T) {
 		{"/get/abc/123abg/test", false, "/get/abc/123abg/:param", Params{Param{Key: "param", Value: "test"}}},
 		{"/get/abc/123abf/testss", false, "/get/abc/123abf/:param", Params{Param{Key: "param", Value: "testss"}}},
 		{"/get/abc/123abfff/te", false, "/get/abc/123abfff/:param", Params{Param{Key: "param", Value: "te"}}},
+		{"/get/abc/escaped_colon/test\\:param", false, "/get/abc/escaped_colon/test\\:param", nil},
 	})
 
 	checkPriorities(t, tree)
@@ -419,6 +421,9 @@ func TestTreeWildcardConflict(t *testing.T) {
 		{"/id/:id", false},
 		{"/static/*file", false},
 		{"/static/", true},
+		{"/escape/test\\:d1", false},
+		{"/escape/test\\:d2", false},
+		{"/escape/test:param", false},
 	}
 	testRoutes(t, routes)
 }
@@ -971,3 +976,20 @@ func TestTreeWildcardConflictEx(t *testing.T) {
 		}
 	}
 }
+
+func TestTreeInvalidEscape(t *testing.T) {
+	routes := map[string]bool{
+		"/r1/r":    true,
+		"/r2/:r":   true,
+		"/r3/\\:r": true,
+	}
+	tree := &node{}
+	for route, valid := range routes {
+		recv := catchPanic(func() {
+			tree.addRoute(route, fakeHandler(route))
+		})
+		if recv == nil != valid {
+			t.Fatalf("%s should be %t but got %v", route, valid, recv)
+		}
+	}
+}