Compare commits

..

7 Commits

Author SHA1 Message Date
Leon cap
be0a30ddf0
Merge 15ef1d4739f1c7765bc3c6979040a584853786e2 into b2b489dbf4826c2c630717a77fd5e42774625410 2026-01-18 18:02:04 -08:00
WeidiDeng
b2b489dbf4
chore(context): always trust xff headers from unix socket (#3359)
Co-authored-by: Bo-Yi Wu <appleboy.tw@gmail.com>
2026-01-18 12:56:22 +08:00
OHZEKI Naoki
3ab698dc51
refactor(recovery): smart error comparison (#4142)
* refactor(recovery): rename var in CustomRecoveryWithWriter

* refactor(recovery): smart error comparison

* test(recovery): Directly reference the syscall error string
2026-01-17 16:40:43 +08:00
Nurysso
9914178584
fix(context): ClientIP handling for multiple X-Forwarded-For header values (#4472)
* Fix ClientIP calculation by concatenating all RemoteIPHeaders values

* test: used http.MethodGet instead constants and fix lints

* lint error fixed

* Refactor ClientIP X-Forwarded-For tests

---------

Co-authored-by: Bo-Yi Wu <appleboy.tw@gmail.com>
2026-01-02 10:15:27 +08:00
Paulo Henrique
915e4c90d2
refactor(context): replace hardcoded localhost IPs with constants (#4481) 2025-12-27 19:25:17 +08:00
Twacqwq
26c3a62865
chore(response): prevent Flush() panic when http.Flusher (#4479) 2025-12-24 18:35:20 +08:00
dependabot[bot]
22c274c84b
chore(deps): bump actions/cache from 4 to 5 in the actions group (#4469)
Bumps the actions group with 1 update: [actions/cache](https://github.com/actions/cache).


Updates `actions/cache` from 4 to 5
- [Release notes](https://github.com/actions/cache/releases)
- [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md)
- [Commits](https://github.com/actions/cache/compare/v4...v5)

---
updated-dependencies:
- dependency-name: actions/cache
  dependency-version: '5'
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: actions
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2025-12-24 18:33:46 +08:00
8 changed files with 98 additions and 41 deletions

View File

@ -65,7 +65,7 @@ jobs:
with:
ref: ${{ github.ref }}
- uses: actions/cache@v4
- uses: actions/cache@v5
with:
path: |
${{ matrix.go-build }}

View File

@ -978,18 +978,32 @@ func (c *Context) ClientIP() string {
}
}
// It also checks if the remoteIP is a trusted proxy or not.
// In order to perform this validation, it will see if the IP is contained within at least one of the CIDR blocks
// defined by Engine.SetTrustedProxies()
remoteIP := net.ParseIP(c.RemoteIP())
if remoteIP == nil {
return ""
var (
trusted bool
remoteIP net.IP
)
// If gin is listening a unix socket, always trust it.
localAddr, ok := c.Request.Context().Value(http.LocalAddrContextKey).(net.Addr)
if ok && strings.HasPrefix(localAddr.Network(), "unix") {
trusted = true
}
// Fallback
if !trusted {
// It also checks if the remoteIP is a trusted proxy or not.
// In order to perform this validation, it will see if the IP is contained within at least one of the CIDR blocks
// defined by Engine.SetTrustedProxies()
remoteIP = net.ParseIP(c.RemoteIP())
if remoteIP == nil {
return ""
}
trusted = c.engine.isTrustedProxy(remoteIP)
}
trusted := c.engine.isTrustedProxy(remoteIP)
if trusted && c.engine.ForwardedByClientIP && c.engine.RemoteIPHeaders != nil {
for _, headerName := range c.engine.RemoteIPHeaders {
ip, valid := c.engine.validateHeader(c.requestHeader(headerName))
headerValue := strings.Join(c.Request.Header.Values(headerName), ",")
ip, valid := c.engine.validateHeader(headerValue)
if valid {
return ip
}

View File

@ -1488,6 +1488,37 @@ func TestContextRenderNoContentIndentedJSON(t *testing.T) {
assert.Equal(t, "application/json; charset=utf-8", w.Header().Get("Content-Type"))
}
func TestContextClientIPWithMultipleHeaders(t *testing.T) {
c, _ := CreateTestContext(httptest.NewRecorder())
c.Request, _ = http.NewRequest(http.MethodGet, "/test", nil)
// Multiple X-Forwarded-For headers
c.Request.Header.Add("X-Forwarded-For", "1.2.3.4, "+localhostIP)
c.Request.Header.Add("X-Forwarded-For", "5.6.7.8")
c.Request.RemoteAddr = localhostIP + ":1234"
c.engine.ForwardedByClientIP = true
c.engine.RemoteIPHeaders = []string{"X-Forwarded-For"}
_ = c.engine.SetTrustedProxies([]string{localhostIP})
// Should return 5.6.7.8 (last non-trusted IP)
assert.Equal(t, "5.6.7.8", c.ClientIP())
}
func TestContextClientIPWithSingleHeader(t *testing.T) {
c, _ := CreateTestContext(httptest.NewRecorder())
c.Request, _ = http.NewRequest(http.MethodGet, "/test", nil)
c.Request.Header.Set("X-Forwarded-For", "1.2.3.4, "+localhostIP)
c.Request.RemoteAddr = localhostIP + ":1234"
c.engine.ForwardedByClientIP = true
c.engine.RemoteIPHeaders = []string{"X-Forwarded-For"}
_ = c.engine.SetTrustedProxies([]string{localhostIP})
// Should return 1.2.3.4
assert.Equal(t, "1.2.3.4", c.ClientIP())
}
// Tests that the response is serialized as Secure JSON
// and Content-Type is set to application/json
func TestContextRenderSecureJSON(t *testing.T) {
@ -2229,6 +2260,16 @@ func TestContextClientIP(t *testing.T) {
c.engine.trustedCIDRs, _ = c.engine.prepareTrustedCIDRs()
resetContextForClientIPTests(c)
// unix address
addr := &net.UnixAddr{Net: "unix", Name: "@"}
c.Request = c.Request.WithContext(context.WithValue(c.Request.Context(), http.LocalAddrContextKey, addr))
c.Request.RemoteAddr = addr.String()
assert.Equal(t, "20.20.20.20", c.ClientIP())
// reset
c.Request = c.Request.WithContext(context.Background())
resetContextForClientIPTests(c)
// Legacy tests (validating that the defaults don't break the
// (insecure!) old behaviour)
assert.Equal(t, "20.20.20.20", c.ClientIP())
@ -2255,7 +2296,7 @@ func TestContextClientIP(t *testing.T) {
resetContextForClientIPTests(c)
// IPv6 support
c.Request.RemoteAddr = "[::1]:12345"
c.Request.RemoteAddr = fmt.Sprintf("[%s]:12345", localhostIPv6)
assert.Equal(t, "20.20.20.20", c.ClientIP())
resetContextForClientIPTests(c)
@ -3593,7 +3634,7 @@ func TestContextCopyShouldNotCancel(t *testing.T) {
}()
addr := strings.Split(l.Addr().String(), ":")
res, err := http.Get(fmt.Sprintf("http://127.0.0.1:%s/", addr[len(addr)-1]))
res, err := http.Get(fmt.Sprintf("http://%s:%s/", localhostIP, addr[len(addr)-1]))
if err != nil {
t.Error(fmt.Errorf("request error: %w", err))
return

View File

@ -83,7 +83,7 @@ func TestLoadHTMLGlobDebugMode(t *testing.T) {
}
func TestH2c(t *testing.T) {
ln, err := net.Listen("tcp", "127.0.0.1:0")
ln, err := net.Listen("tcp", localhostIP+":0")
if err != nil {
t.Error(err)
}

View File

@ -12,12 +12,12 @@ import (
"fmt"
"io"
"log"
"net"
"net/http"
"net/http/httputil"
"os"
"runtime"
"strings"
"syscall"
"time"
"github.com/gin-gonic/gin/internal/bytesconv"
@ -57,40 +57,33 @@ func CustomRecoveryWithWriter(out io.Writer, handle RecoveryFunc) HandlerFunc {
}
return func(c *Context) {
defer func() {
if err := recover(); err != nil {
if rec := recover(); rec != nil {
// Check for a broken connection, as it is not really a
// condition that warrants a panic stack trace.
var brokenPipe bool
if ne, ok := err.(*net.OpError); ok {
var se *os.SyscallError
if errors.As(ne, &se) {
seStr := strings.ToLower(se.Error())
if strings.Contains(seStr, "broken pipe") ||
strings.Contains(seStr, "connection reset by peer") {
brokenPipe = true
}
}
}
if e, ok := err.(error); ok && errors.Is(e, http.ErrAbortHandler) {
brokenPipe = true
var isBrokenPipe bool
err, ok := rec.(error)
if ok {
isBrokenPipe = errors.Is(err, syscall.EPIPE) ||
errors.Is(err, syscall.ECONNRESET) ||
errors.Is(err, http.ErrAbortHandler)
}
if logger != nil {
if brokenPipe {
logger.Printf("%s\n%s%s", err, secureRequestDump(c.Request), reset)
if isBrokenPipe {
logger.Printf("%s\n%s%s", rec, secureRequestDump(c.Request), reset)
} else if IsDebugging() {
logger.Printf("[Recovery] %s panic recovered:\n%s\n%s\n%s%s",
timeFormat(time.Now()), secureRequestDump(c.Request), err, stack(stackSkip), reset)
timeFormat(time.Now()), secureRequestDump(c.Request), rec, stack(stackSkip), reset)
} else {
logger.Printf("[Recovery] %s panic recovered:\n%s\n%s%s",
timeFormat(time.Now()), err, stack(stackSkip), reset)
timeFormat(time.Now()), rec, stack(stackSkip), reset)
}
}
if brokenPipe {
if isBrokenPipe {
// If the connection is dead, we can't write a status to it.
c.Error(err.(error)) //nolint: errcheck
c.Error(err) //nolint: errcheck
c.Abort()
} else {
handle(c, err)
handle(c, rec)
}
}
}()

View File

@ -98,13 +98,13 @@ func TestFunction(t *testing.T) {
func TestPanicWithBrokenPipe(t *testing.T) {
const expectCode = 204
expectMsgs := map[syscall.Errno]string{
syscall.EPIPE: "broken pipe",
syscall.ECONNRESET: "connection reset by peer",
expectErrnos := []syscall.Errno{
syscall.EPIPE,
syscall.ECONNRESET,
}
for errno, expectMsg := range expectMsgs {
t.Run(expectMsg, func(t *testing.T) {
for _, errno := range expectErrnos {
t.Run("Recovery from "+errno.Error(), func(t *testing.T) {
var buf strings.Builder
router := New()
@ -122,7 +122,8 @@ func TestPanicWithBrokenPipe(t *testing.T) {
w := PerformRequest(router, http.MethodGet, "/recovery")
// TEST
assert.Equal(t, expectCode, w.Code)
assert.Contains(t, strings.ToLower(buf.String()), expectMsg)
assert.Contains(t, strings.ToLower(buf.String()), errno.Error())
assert.NotContains(t, strings.ToLower(buf.String()), "[Recovery]")
})
}
}

View File

@ -128,7 +128,9 @@ func (w *responseWriter) CloseNotify() <-chan bool {
// Flush implements the http.Flusher interface.
func (w *responseWriter) Flush() {
w.WriteHeaderNow()
w.ResponseWriter.(http.Flusher).Flush()
if f, ok := w.ResponseWriter.(http.Flusher); ok {
f.Flush()
}
}
func (w *responseWriter) Pusher() (pusher http.Pusher) {

View File

@ -19,6 +19,12 @@ import (
// BindKey indicates a default bind key.
const BindKey = "_gin-gonic/gin/bindkey"
// localhostIP indicates the default localhost IP address.
const localhostIP = "127.0.0.1"
// localhostIPv6 indicates the default localhost IPv6 address.
const localhostIPv6 = "::1"
// Bind is a helper function for given interface object and returns a Gin middleware.
func Bind(val any) HandlerFunc {
value := reflect.ValueOf(val)