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

gin.Context with fallback value from gin.Context.Request.Context() #2751

Merged
merged 6 commits into from Jun 24, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE.md
@@ -1,7 +1,7 @@
- With pull requests:
- Open your pull request against `master`
- Your pull request should have no more than two commits, if not you should squash them.
- It should pass all tests in the available continuous integration systems such as TravisCI.
- It should pass all tests in the available continuous integration systems such as GitHub Actions.
- You should add/modify tests to cover your proposed code changes.
- If your pull request contains a new feature, please document it on the README.

62 changes: 62 additions & 0 deletions .github/workflows/gin.yml
@@ -0,0 +1,62 @@
name: Run Tests

on:
push:
branches:
- master
pull_request:
branches:
- master

jobs:
test:
strategy:
matrix:
os: [ubuntu-latest, macos-latest]
go: [1.13, 1.14, 1.15, 1.16]
test-tags: ['', nomsgpack]
name: ${{ matrix.os }} @ Go ${{ matrix.go }} ${{ matrix.test-tags }}
runs-on: ${{ matrix.os }}
env:
GO111MODULE: on
TESTTAGS: ${{ matrix.test-tags }}
GOPROXY: https://proxy.golang.org
steps:
- name: Set up Go ${{ matrix.go }}
uses: actions/setup-go@v2
with:
go-version: ${{ matrix.go }}

- name: Checkout Code
uses: actions/checkout@v2
with:
ref: ${{ github.ref }}

- name: Install Dependencies
run: make tools

- name: Run Check
run: |
make vet
make fmt-check
make misspell-check

- name: Run Tests
run: make test

- name: Upload coverage to Codecov
uses: codecov/codecov-action@v1
notification-gitter:
needs: test
runs-on: ubuntu-latest
steps:
- name: Notification failure message
if: failure()
run: |
PR_OR_COMPARE="$(if [ "${{ github.event.pull_request }}" != "" ]; then echo "${{ github.event.pull_request.html_url }}"; else echo "${{ github.event.compare }}"; fi)"
curl -d message="GitHub Actions [$GITHUB_REPOSITORY]($PR_OR_COMPARE) ($GITHUB_REF) [normal]($GITHUB_API_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID) ($GITHUB_RUN_NUMBER)" -d level=error https://webhooks.gitter.im/e/7f95bf605c4d356372f4
- name: Notification success message
if: success()
run: |
PR_OR_COMPARE="$(if [ "${{ github.event.pull_request }}" != "" ]; then echo "${{ github.event.pull_request.html_url }}"; else echo "${{ github.event.compare }}"; fi)"
curl -d message="GitHub Actions [$GITHUB_REPOSITORY]($PR_OR_COMPARE) ($GITHUB_REF) [normal]($GITHUB_API_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID) ($GITHUB_RUN_NUMBER)" https://webhooks.gitter.im/e/7f95bf605c4d356372f4
52 changes: 0 additions & 52 deletions .travis.yml

This file was deleted.

2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Expand Up @@ -8,6 +8,6 @@
- With pull requests:
- Open your pull request against `master`
- Your pull request should have no more than two commits, if not you should squash them.
- It should pass all tests in the available continuous integration systems such as TravisCI.
- It should pass all tests in the available continuous integration systems such as GitHub Actions.
- You should add/modify tests to cover your proposed code changes.
- If your pull request contains a new feature, please document it on the README.
10 changes: 8 additions & 2 deletions Makefile
@@ -1,5 +1,6 @@
GO ?= go
GOFMT ?= gofmt "-s"
GO_VERSION=$(shell $(GO) version | cut -c 14- | cut -d' ' -f1 | cut -d'.' -f2)
PACKAGES ?= $(shell $(GO) list ./...)
VETPACKAGES ?= $(shell $(GO) list ./... | grep -v /examples/)
GOFILES := $(shell find . -name "*.go")
Expand Down Expand Up @@ -67,5 +68,10 @@ misspell:

.PHONY: tools
tools:
go install golang.org/x/lint/golint; \
go install github.com/client9/misspell/cmd/misspell;
@if [ $(GO_VERSION) -gt 15 ]; then \
$(GO) install golang.org/x/lint/golint@latest; \
$(GO) install github.com/client9/misspell/cmd/misspell@latest; \
elif [ $(GO_VERSION) -lt 16 ]; then \
$(GO) install golang.org/x/lint/golint; \
$(GO) install github.com/client9/misspell/cmd/misspell; \
fi
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -2,7 +2,7 @@

<img align="right" width="159px" src="https://raw.githubusercontent.com/gin-gonic/logo/master/color.png">

[![Build Status](https://travis-ci.org/gin-gonic/gin.svg)](https://travis-ci.org/gin-gonic/gin)
[![Build Status](https://github.com/gin-gonic/gin/workflows/Run%20Tests/badge.svg?branch=master)](https://github.com/gin-gonic/gin/actions?query=branch%3Amaster)
[![codecov](https://codecov.io/gh/gin-gonic/gin/branch/master/graph/badge.svg)](https://codecov.io/gh/gin-gonic/gin)
[![Go Report Card](https://goreportcard.com/badge/github.com/gin-gonic/gin)](https://goreportcard.com/report/github.com/gin-gonic/gin)
[![GoDoc](https://pkg.go.dev/badge/github.com/gin-gonic/gin?status.svg)](https://pkg.go.dev/github.com/gin-gonic/gin?tab=doc)
Expand Down
26 changes: 20 additions & 6 deletions context.go
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"io"
"io/ioutil"
"log"
"math"
"mime/multipart"
"net"
Expand Down Expand Up @@ -731,17 +732,26 @@ func (c *Context) ShouldBindBodyWith(obj interface{}, bb binding.BindingBody) (e
// If the headers are nots syntactically valid OR the remote IP does not correspong to a trusted proxy,
// the remote IP (coming form Request.RemoteAddr) is returned.
func (c *Context) ClientIP() string {
switch {
case c.engine.AppEngine:
// Check if we're running on a tursted platform
switch c.engine.TrustedPlatform {
case PlatformGoogleAppEngine:
if addr := c.requestHeader("X-Appengine-Remote-Addr"); addr != "" {
return addr
}
case c.engine.CloudflareProxy:
case PlatformCloudflare:
if addr := c.requestHeader("CF-Connecting-IP"); addr != "" {
return addr
}
}

// Legacy "AppEngine" flag
if c.engine.AppEngine {
log.Println(`The AppEngine flag is going to be deprecated. Please check issues #2723 and #2739 and use 'TrustedPlatform: gin.PlatformGoogleAppEngine' instead.`)
if addr := c.requestHeader("X-Appengine-Remote-Addr"); addr != "" {
return addr
}
}

remoteIP, trusted := c.RemoteIP()
if remoteIP == nil {
return ""
Expand Down Expand Up @@ -1176,8 +1186,12 @@ func (c *Context) Value(key interface{}) interface{} {
return c.Request
}
if keyAsString, ok := key.(string); ok {
val, _ := c.Get(keyAsString)
return val
if val, exists := c.Get(keyAsString); exists {
return val
}
}
return nil
if c.Request == nil || c.Request.Context() == nil {
wei840222 marked this conversation as resolved.
Show resolved Hide resolved
return nil
}
return c.Request.Context().Value(key)
}
2 changes: 1 addition & 1 deletion context_appengine.go
Expand Up @@ -8,5 +8,5 @@
package gin

func init() {
defaultAppEngine = true
defaultPlatform = PlatformGoogleAppEngine
}
29 changes: 26 additions & 3 deletions context_test.go
Expand Up @@ -1410,7 +1410,7 @@ func TestContextClientIP(t *testing.T) {

c.Request.Header.Del("X-Forwarded-For")
c.Request.Header.Del("X-Real-IP")
c.engine.AppEngine = true
c.engine.TrustedPlatform = PlatformGoogleAppEngine
assert.Equal(t, "50.50.50.50", c.ClientIP())

c.Request.Header.Del("X-Appengine-Remote-Addr")
Expand Down Expand Up @@ -1470,19 +1470,27 @@ func TestContextClientIP(t *testing.T) {
assert.Equal(t, "10.10.10.10", c.ClientIP())

c.engine.RemoteIPHeaders = []string{}
c.engine.TrustedPlatform = PlatformGoogleAppEngine
assert.Equal(t, "50.50.50.50", c.ClientIP())

// Test the legacy flag
c.engine.TrustedPlatform = ""
c.engine.AppEngine = true
assert.Equal(t, "50.50.50.50", c.ClientIP())
c.engine.AppEngine = false
c.engine.TrustedPlatform = PlatformGoogleAppEngine

c.Request.Header.Del("X-Appengine-Remote-Addr")
assert.Equal(t, "40.40.40.40", c.ClientIP())

c.engine.AppEngine = false
c.engine.CloudflareProxy = true
c.engine.TrustedPlatform = PlatformCloudflare
assert.Equal(t, "60.60.60.60", c.ClientIP())

c.Request.Header.Del("CF-Connecting-IP")
assert.Equal(t, "40.40.40.40", c.ClientIP())

c.engine.TrustedPlatform = ""

// no port
c.Request.RemoteAddr = "50.50.50.50"
assert.Empty(t, c.ClientIP())
Expand All @@ -1494,6 +1502,7 @@ func resetContextForClientIPTests(c *Context) {
c.Request.Header.Set("X-Appengine-Remote-Addr", "50.50.50.50")
c.Request.Header.Set("CF-Connecting-IP", "60.60.60.60")
c.Request.RemoteAddr = " 40.40.40.40:42123 "
c.engine.TrustedPlatform = ""
c.engine.AppEngine = false
}

Expand Down Expand Up @@ -2048,3 +2057,17 @@ func TestRemoteIPFail(t *testing.T) {
assert.Nil(t, ip)
assert.False(t, trust)
}

func TestContextWithFallbackValueFromRequestContext(t *testing.T) {
var key struct{}
c := &Context{}
c.Request, _ = http.NewRequest("POST", "/", nil)
c.Request = c.Request.WithContext(context.WithValue(context.TODO(), key, "value"))

assert.Equal(t, "value", c.Value(key))

c2 := &Context{}
c2.Request, _ = http.NewRequest("POST", "/", nil)
c2.Request = c2.Request.WithContext(context.WithValue(context.TODO(), "key", "value2"))
assert.Equal(t, "value2", c2.Value("key"))
}
23 changes: 17 additions & 6 deletions gin.go
Expand Up @@ -25,7 +25,7 @@ var (
default405Body = []byte("405 method not allowed")
)

var defaultAppEngine bool
var defaultPlatform string

// HandlerFunc defines the handler used by gin middleware as return value.
type HandlerFunc func(*Context)
Expand All @@ -52,6 +52,16 @@ type RouteInfo struct {
// RoutesInfo defines a RouteInfo array.
type RoutesInfo []RouteInfo

// Trusted platforms
const (
// When running on Google App Engine. Trust X-Appengine-Remote-Addr
// for determining the client's IP
PlatformGoogleAppEngine = "google-app-engine"
// When using Cloudflare's CDN. Trust CF-Connecting-IP for determining
// the client's IP
PlatformCloudflare = "cloudflare"
)

// Engine is the framework's instance, it contains the muxer, middleware and configuration settings.
// Create an instance of Engine, by using New() or Default()
type Engine struct {
Expand Down Expand Up @@ -101,14 +111,15 @@ type Engine struct {
// `true`.
TrustedProxies []string

// If set to a constant of value gin.Platform*, trusts the headers set by
// that platform, for example to determine the client IP
TrustedPlatform string

// DEPRECATED: USE `TrustedPlatform` WITH VALUE `gin.GoogleAppEngine` INSTEAD
// #726 #755 If enabled, it will trust some headers starting with
// 'X-AppEngine...' for better integration with that PaaS.
AppEngine bool

// If enabled, it will trust the CF-Connecting-IP header to determine the
// IP of the client.
CloudflareProxy bool

// If enabled, the url.RawPath will be used to find parameters.
UseRawPath bool

Expand Down Expand Up @@ -164,7 +175,7 @@ func New() *Engine {
ForwardedByClientIP: true,
RemoteIPHeaders: []string{"X-Forwarded-For", "X-Real-IP"},
TrustedProxies: []string{"0.0.0.0/0"},
AppEngine: defaultAppEngine,
TrustedPlatform: defaultPlatform,
UseRawPath: false,
RemoveExtraSlash: false,
UnescapePathValues: true,
Expand Down
8 changes: 4 additions & 4 deletions tree.go
Expand Up @@ -30,8 +30,8 @@ type Param struct {
// It is therefore safe to read values by the index.
type Params []Param

// Get returns the value of the first Param which key matches the given name.
// If no matching Param is found, an empty string is returned.
// Get returns the value of the first Param which key matches the given name and a boolean true.
// If no matching Param is found, an empty string is returned and a boolean false .
func (ps Params) Get(name string) (string, bool) {
for _, entry := range ps {
if entry.Key == name {
Expand Down Expand Up @@ -487,7 +487,7 @@ walk: // Outer loop for walking the tree
}

// ... but we can't
value.tsr = (len(path) == end+1)
value.tsr = len(path) == end+1
return
}

Expand All @@ -499,7 +499,7 @@ walk: // Outer loop for walking the tree
// No handle found. Check if a handle for this path + a
// trailing slash exists for TSR recommendation
n = n.children[0]
value.tsr = (n.path == "/" && n.handlers != nil)
value.tsr = n.path == "/" && n.handlers != nil
}
return

Expand Down