Merge branch 'master' into main

This commit is contained in:
Raju Ahmed 2026-01-21 09:27:53 +06:00 committed by GitHub
commit 49a0f4a7a4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 85 additions and 36 deletions

View File

@ -988,18 +988,32 @@ func (c *Context) ClientIP() string {
} }
} }
// It also checks if the remoteIP is a trusted proxy or not. var (
// In order to perform this validation, it will see if the IP is contained within at least one of the CIDR blocks trusted bool
// defined by Engine.SetTrustedProxies() remoteIP net.IP
remoteIP := net.ParseIP(c.RemoteIP()) )
if remoteIP == nil { // If gin is listening a unix socket, always trust it.
return "" 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 { if trusted && c.engine.ForwardedByClientIP && c.engine.RemoteIPHeaders != nil {
for _, headerName := range c.engine.RemoteIPHeaders { 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 { if valid {
return ip return ip
} }

View File

@ -1165,6 +1165,37 @@ func TestContextRenderNoContentIndentedJSON(t *testing.T) {
assert.Equal(t, "application/json; charset=utf-8", w.Header().Get("Content-Type")) 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 // Tests that the response is serialized as Secure JSON
// and Content-Type is set to application/json // and Content-Type is set to application/json
func TestContextRenderSecureJSON(t *testing.T) { func TestContextRenderSecureJSON(t *testing.T) {
@ -1906,6 +1937,16 @@ func TestContextClientIP(t *testing.T) {
c.engine.trustedCIDRs, _ = c.engine.prepareTrustedCIDRs() c.engine.trustedCIDRs, _ = c.engine.prepareTrustedCIDRs()
resetContextForClientIPTests(c) 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 // Legacy tests (validating that the defaults don't break the
// (insecure!) old behaviour) // (insecure!) old behaviour)
assert.Equal(t, "20.20.20.20", c.ClientIP()) assert.Equal(t, "20.20.20.20", c.ClientIP())

View File

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

View File

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