Repository: gin-gonic/gin
Pull request #2832 (2021-08-19 18:00:19)

Quick Fix c.ClientIP() mistakely parsing to 127.0.0.1 for who not using r.Run() to run http server
Related issues: #2791 #2697 #2814 

Someone who not using `r.Run()` to run http server may meet this issue: `c.ClientIP()` always returns the same IP for all clients because of calling of `prepareTrustedCIDRs()` of `TrustedProxies` feature.
For example, `c.ClientIP()` always returns `127.0.0.1` while using local reverse proxy.


In `New()`: there is one definition: `TrustedProxies: []string{"0.0.0.0/0"}`
We can see this variable has default value, but `trustedCIDRs` doesn't have.

So we just need to give a default value to `trustedCIDRs`.

codecov[bot]: # [Codecov](https://codecov.io/gh/gin-gonic/gin/pull/2832?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) Report
> Merging [#2832](https://codecov.io/gh/gin-gonic/gin/pull/2832?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) (bd77f6a) into [master](https://codecov.io/gh/gin-gonic/gin/commit/a46dee3a9af4edd9de1694e3ba3592b20c6a8968?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) (a46dee3) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/gin-gonic/gin/pull/2832/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/2832?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    #2832   +/-   ##
=======================================
  Coverage   98.75%   98.75%           
=======================================
  Files          41       41           
  Lines        3063     3064    +1     
=======================================
+ Hits         3025     3026    +1     
  Misses         26       26           
  Partials       12       12           
```

| Flag | Coverage Δ | |
|---|---|---|
| go-1.13 | `98.74% <100.00%> (-0.02%)` | :arrow_down: |
| go-1.14 | `?` | |
| go-1.15 | `98.58% <100.00%> (-0.01%)` | :arrow_down: |
| go-1.16 | `98.57% <100.00%> (-0.02%)` | :arrow_down: |
| macos-latest | `?` | |
| nomsgpack | `98.74% <100.00%> (+<0.01%)` | :arrow_up: |
| ubuntu-latest | `98.75% <100.00%> (+<0.01%)` | :arrow_up: |

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/2832?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/2832/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic#diff-Z2luLmdv) | `99.10% <100.00%> (+<0.01%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/gin-gonic/gin/pull/2832?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/2832?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic). Last update [a46dee3...bd77f6a](https://codecov.io/gh/gin-gonic/gin/pull/2832?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).

Bisstocuz: @menduo @zihengCat Please leave your review here to help fix this as soon as possible.

This change will parse TrustedProxies and trust all proxies by default.
Bisstocuz: @thinkerou @appleboy code review plz
appleboy: @Bisstocuz I gave the comment.
Bisstocuz: @junikaii @menduo I think that directly give it a absolute default value is more consistent.
Bisstocuz: @appleboy Hi, code review plz.

After this PR merged, I will do some works and create a new PR to perfect `TrustedProxies` feature according to this:

> This fix has 2 problems:
>
>1. it's not backward-compatible
>2. is doesn't update related documents

https://github.com/gin-gonic/gin/pull/2692#issuecomment-881370810 by @leafduo 
appleboy: @thinkerou need your approval.
zihengCat: @Bisstocuz Could you make all CI unit tests passed?
Bisstocuz: > @Bisstocuz Could you make all CI unit tests passed?

@zihengCat I'm checking it.
Bisstocuz: @appleboy @zihengCat @thinkerou @junikaii 
Hi, code review plz again, #2872 broke unit tests, and I fixed it now.

Test result: https://github.com/Bisstocuz/gin/actions/runs/1285966832

Patch:
diff --git a/gin.go b/gin.go
index ae6d479..02a1062 100644
--- a/gin.go
+++ b/gin.go
@@ -183,6 +183,7 @@ func New() *Engine {
 		trees:                  make(methodTrees, 0, 9),
 		delims:                 render.Delims{Left: "{{", Right: "}}"},
 		secureJSONPrefix:       "while(1);",
+		trustedCIDRs:           []*net.IPNet{{IP: net.IP{0x0, 0x0, 0x0, 0x0}, Mask: net.IPMask{0x0, 0x0, 0x0, 0x0}}},
 	}
 	engine.RouterGroup.engine = engine
 	engine.pool.New = func() interface{} {
@@ -264,9 +265,8 @@ func (engine *Engine) NoRoute(handlers ...HandlerFunc) {
 	engine.rebuild404Handlers()
 }
 
-// NoMethod sets the handlers called when NoMethod.
+// NoMethod sets the handlers called when Engine.HandleMethodNotAllowed = true.
 func (engine *Engine) NoMethod(handlers ...HandlerFunc) {
-	engine.HandleMethodNotAllowed = true
 	engine.noMethod = handlers
 	engine.rebuild405Handlers()
 }


Test patch:
diff --git a/middleware_test.go b/middleware_test.go
index fca1c53..4b4afd4 100644
--- a/middleware_test.go
+++ b/middleware_test.go
@@ -118,7 +118,10 @@ func TestMiddlewareNoMethodEnabled(t *testing.T) {
 func TestMiddlewareNoMethodDisabled(t *testing.T) {
 	signature := ""
 	router := New()
+
+	// NoMethod disabled
 	router.HandleMethodNotAllowed = false
+
 	router.Use(func(c *Context) {
 		signature += "A"
 		c.Next()
@@ -144,6 +147,7 @@ func TestMiddlewareNoMethodDisabled(t *testing.T) {
 	router.POST("/", func(c *Context) {
 		signature += " XX "
 	})
+
 	// RUN
 	w := performRequest(router, "GET", "/")