Repository: mccutchen/go-httpbin
Pull request #139 (2023-07-21 04:49:27)

feat: allow filtering incoming request headers
I want to deploy go-httpbin to Alibaba Cloud Function Compute (A Serverless Runtime), But in FC, the Server will add some custom header on the request headers:

for example:
![CleanShot 2023-07-21 at 12 50 06@2x](https://github.com/mccutchen/go-httpbin/assets/13938334/c2dd08a1-3a8a-4e72-a421-645c4321ab34)


So I add this logic to process unused headers:

```golang
	opts := []httpbin.OptionFunc{
		httpbin.WithMaxBodySize(cfg.MaxBodySize),
		httpbin.WithMaxDuration(cfg.MaxDuration),
		httpbin.WithObserver(httpbin.StdLogObserver(logger)),
		httpbin.WithExcludeHeaders("x-ignore-*,x-info-this-key"),
	}
```


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/mccutchen/go-httpbin/pull/139?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen) Report
> Merging [#139](https://app.codecov.io/gh/mccutchen/go-httpbin/pull/139?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen) (1bfbdb8) into [main](https://app.codecov.io/gh/mccutchen/go-httpbin/commit/3761c4ae98ddc23554d61813dbd688c7318737bf?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen) (3761c4a) will **decrease** coverage by `0.06%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main     #139      +/-   ##
==========================================
- Coverage   98.23%   98.17%   -0.06%     
==========================================
  Files           8        8              
  Lines        1527     1534       +7     
==========================================
+ Hits         1500     1506       +6     
- Misses         19       20       +1     
  Partials        8        8              
```

| [Impacted Files](https://app.codecov.io/gh/mccutchen/go-httpbin/pull/139?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen) | Coverage Δ | |
|---|---|---|
| [httpbin/httpbin.go](https://app.codecov.io/gh/mccutchen/go-httpbin/pull/139?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen#diff-aHR0cGJpbi9odHRwYmluLmdv) | `100.00% <ø> (ø)` | |
| [httpbin/handlers.go](https://app.codecov.io/gh/mccutchen/go-httpbin/pull/139?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen#diff-aHR0cGJpbi9oYW5kbGVycy5nbw==) | `99.45% <100.00%> (ø)` | |
| [httpbin/helpers.go](https://app.codecov.io/gh/mccutchen/go-httpbin/pull/139?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen#diff-aHR0cGJpbi9oZWxwZXJzLmdv) | `96.08% <100.00%> (-0.32%)` | :arrow_down: |
| [httpbin/options.go](https://app.codecov.io/gh/mccutchen/go-httpbin/pull/139?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen#diff-aHR0cGJpbi9vcHRpb25zLmdv) | `100.00% <100.00%> (ø)` | |

mccutchen: Ah, interesting idea.  Given that using this new Interceptor will require wiring up your own go-httpbin server rather than just running a prebuilt binary or docker image, I should point out that the same thing could be accomplished via plain old stlib HTTP handler middleware without requiring any changes to go-httpbin.  (I'd be happy to show a specific example, if it would help!)

It seems like the real value would be exposing this as configuration, to make it easy for anyone to deploy a "stock" go-httpbin image and still drop platform-specific headers.  I can imagine being able to do something like this (including wildcard prefix matches) being generally useful:

```
-exclude-headers="Foo,Bar,X-Fc-*"
EXCLUDE_HEADERS="Foo,Bar,X-Fc-*"
```

Do you have any interest in adapting this work to that end?  (If so, I'd consider simplifying the implementation to just deal with headers.  The notion of flexible "Interceptors" is nice, but feels like a bit more than we need here.)
bytemain: Yes, It seem that expose this feature as a configuration is more reasonable.

But in my opinion, I think the Interceptor mechanism should be kept, it make the whole httpbin more configurable. It is actually a hook system,  bring some hooks so that we can configure by ourself.

In the first version PR, I just added a very simple interceptor implementation(I am lazy, XD), I think the Observer is the same as Interceptor, So can I still use the "interceptor"?

mccutchen: What other interceptors do you imagine being useful here?  Maybe it's a lack of imagination on my part, but I do not think we'd want to support anything other than modifying _incoming_ request headers, and I can't think of any reason to do anything aside from dropping them.

To do that, a fancy system of interceptor hooks is not required.  It can be implemented as a fairly simple middleware.

(I'd put the Observer in a different category, simply because golang's stdlib makes it difficult/awkward to instrument outgoing HTTP _responses_.  There, the Observer abstraction makes it possible for go-httpbin to surface details about the generated response in a way that is easy for 3rd parties to build on.  No such abstraction is necessary for filtering out incoming request headers.)
bytemain: @mccutchen I have simplified the implementation and add configuration ability~
bytemain: Thanks for your advice, I have done for them.

And now the headerProcessor will only be used if user configured `exclude-headers` configuration.

Patch:
diff --git a/README.md b/README.md
index 91227a0..9d3d9a5 100644
--- a/README.md
+++ b/README.md
@@ -91,6 +91,7 @@ variables (or a combination of the two):
 | `-max-duration` | `MAX_DURATION` | Maximum duration a response may take | 10s |
 | `-port` | `PORT` | Port to listen on | 8080 |
 | `-use-real-hostname` | `USE_REAL_HOSTNAME` | Expose real hostname as reported by os.Hostname() in the /hostname endpoint | false |
+| `-exclude-headers` | `EXCLUDE_HEADERS` | Drop platform-specific headers. Comma-separated list of headers key to drop, supporting wildcard suffix matching. For example: `"foo,bar,x-fc-*"` | - |
 
 **Notes:**
 - Command line arguments take precedence over environment variables.
@@ -159,6 +160,13 @@ public internet, consider tuning it appropriately:
      logging using [zerolog] and further hardens the HTTP server against
      malicious clients by tuning lower-level timeouts and limits.
 
+5. **Prevent leaking sensitive headers**
+
+  By default, go-httpbin will return any headers sent by the client in the response.
+  But if you want to deploy go-httpbin in some serverless environment, you may want to drop some headers.
+  You can use the `-exclude-headers` CLI argument or the `EXCLUDE_HEADERS` env var to configure an appropriate allowlist.
+  For example, Alibaba Cloud Function Compute will [add some headers like `x-fc-*` to the request](https://www.alibabacloud.com/help/en/fc/user-guide/specification-details). if you want to drop these `x-fc-*` headers, you can set `EXCLUDE_HEADERS=x-fc-*`.
+
 ## Development
 
 ```bash
diff --git a/httpbin/cmd/cmd.go b/httpbin/cmd/cmd.go
index c006dc0..1a1b709 100644
--- a/httpbin/cmd/cmd.go
+++ b/httpbin/cmd/cmd.go
@@ -69,6 +69,7 @@ func mainImpl(args []string, getEnv func(string) string, getHostname func() (str
 		httpbin.WithMaxBodySize(cfg.MaxBodySize),
 		httpbin.WithMaxDuration(cfg.MaxDuration),
 		httpbin.WithObserver(httpbin.StdLogObserver(logger)),
+		httpbin.WithExcludeHeaders(cfg.ExcludeHeaders),
 	}
 	if cfg.RealHostname != "" {
 		opts = append(opts, httpbin.WithHostname(cfg.RealHostname))
@@ -99,6 +100,7 @@ func mainImpl(args []string, getEnv func(string) string, getHostname func() (str
 type config struct {
 	AllowedRedirectDomains []string
 	ListenHost             string
+	ExcludeHeaders         string
 	ListenPort             int
 	MaxBodySize            int64
 	MaxDuration            time.Duration
@@ -140,6 +142,7 @@ func loadConfig(args []string, getEnv func(string) string, getHostname func() (s
 	fs.StringVar(&cfg.ListenHost, "host", defaultListenHost, "Host to listen on")
 	fs.StringVar(&cfg.TLSCertFile, "https-cert-file", "", "HTTPS Server certificate file")
 	fs.StringVar(&cfg.TLSKeyFile, "https-key-file", "", "HTTPS Server private key file")
+	fs.StringVar(&cfg.ExcludeHeaders, "exclude-headers", "", "Drop platform-specific headers. Comma-separated list of headers key to drop, supporting wildcard matching.")
 
 	// in order to fully control error output whether CLI arguments or env vars
 	// are used to configure the app, we need to take control away from the
@@ -189,6 +192,9 @@ func loadConfig(args []string, getEnv func(string) string, getHostname func() (s
 	if cfg.ListenHost == defaultListenHost && getEnv("HOST") != "" {
 		cfg.ListenHost = getEnv("HOST")
 	}
+	if cfg.ExcludeHeaders == "" && getEnv("EXCLUDE_HEADERS") != "" {
+		cfg.ExcludeHeaders = getEnv("EXCLUDE_HEADERS")
+	}
 	if cfg.ListenPort == defaultListenPort && getEnv("PORT") != "" {
 		cfg.ListenPort, err = strconv.Atoi(getEnv("PORT"))
 		if err != nil {
diff --git a/httpbin/handlers.go b/httpbin/handlers.go
index f7cfea7..0755a83 100644
--- a/httpbin/handlers.go
+++ b/httpbin/handlers.go
@@ -47,7 +47,7 @@ func (h *HTTPBin) UTF8(w http.ResponseWriter, r *http.Request) {
 func (h *HTTPBin) Get(w http.ResponseWriter, r *http.Request) {
 	writeJSON(http.StatusOK, w, &noBodyResponse{
 		Args:    r.URL.Query(),
-		Headers: getRequestHeaders(r),
+		Headers: getRequestHeaders(r, h.excludeHeadersProcessor),
 		Method:  r.Method,
 		Origin:  getClientIP(r),
 		URL:     getURL(r).String(),
@@ -74,7 +74,7 @@ func (h *HTTPBin) RequestWithBody(w http.ResponseWriter, r *http.Request) {
 		Args:    r.URL.Query(),
 		Files:   nilValues,
 		Form:    nilValues,
-		Headers: getRequestHeaders(r),
+		Headers: getRequestHeaders(r, h.excludeHeadersProcessor),
 		Method:  r.Method,
 		Origin:  getClientIP(r),
 		URL:     getURL(r).String(),
@@ -96,7 +96,7 @@ func (h *HTTPBin) Gzip(w http.ResponseWriter, r *http.Request) {
 	)
 	mustMarshalJSON(gzw, &noBodyResponse{
 		Args:    r.URL.Query(),
-		Headers: getRequestHeaders(r),
+		Headers: getRequestHeaders(r, h.excludeHeadersProcessor),
 		Method:  r.Method,
 		Origin:  getClientIP(r),
 		Gzipped: true,
@@ -119,7 +119,7 @@ func (h *HTTPBin) Deflate(w http.ResponseWriter, r *http.Request) {
 	)
 	mustMarshalJSON(zw, &noBodyResponse{
 		Args:     r.URL.Query(),
-		Headers:  getRequestHeaders(r),
+		Headers:  getRequestHeaders(r, h.excludeHeadersProcessor),
 		Method:   r.Method,
 		Origin:   getClientIP(r),
 		Deflated: true,
@@ -151,7 +151,7 @@ func (h *HTTPBin) UserAgent(w http.ResponseWriter, r *http.Request) {
 // Headers echoes the incoming request headers
 func (h *HTTPBin) Headers(w http.ResponseWriter, r *http.Request) {
 	writeJSON(http.StatusOK, w, &headersResponse{
-		Headers: getRequestHeaders(r),
+		Headers: getRequestHeaders(r, h.excludeHeadersProcessor),
 	})
 }
 
@@ -538,7 +538,7 @@ func (h *HTTPBin) Stream(w http.ResponseWriter, r *http.Request) {
 
 	resp := &streamResponse{
 		Args:    r.URL.Query(),
-		Headers: getRequestHeaders(r),
+		Headers: getRequestHeaders(r, h.excludeHeadersProcessor),
 		Origin:  getClientIP(r),
 		URL:     getURL(r).String(),
 	}
@@ -783,7 +783,7 @@ func (h *HTTPBin) ETag(w http.ResponseWriter, r *http.Request) {
 	var buf bytes.Buffer
 	mustMarshalJSON(&buf, noBodyResponse{
 		Args:    r.URL.Query(),
-		Headers: getRequestHeaders(r),
+		Headers: getRequestHeaders(r, h.excludeHeadersProcessor),
 		Method:  r.Method,
 		Origin:  getClientIP(r),
 		URL:     getURL(r).String(),
diff --git a/httpbin/helpers.go b/httpbin/helpers.go
index 76db70b..8b04ca6 100644
--- a/httpbin/helpers.go
+++ b/httpbin/helpers.go
@@ -13,6 +13,7 @@ import (
 	"mime/multipart"
 	"net/http"
 	"net/url"
+	"regexp"
 	"strconv"
 	"strings"
 	"sync"
@@ -28,12 +29,15 @@ const Base64MaxLen = 2000
 // This is necessary to ensure that the incoming Host and Transfer-Encoding
 // headers are included, because golang only exposes those values on the
 // http.Request struct itself.
-func getRequestHeaders(r *http.Request) http.Header {
+func getRequestHeaders(r *http.Request, fn headersProcessorFunc) http.Header {
 	h := r.Header
 	h.Set("Host", r.Host)
 	if len(r.TransferEncoding) > 0 {
 		h.Set("Transfer-Encoding", strings.Join(r.TransferEncoding, ","))
 	}
+	if fn != nil {
+		return fn(h)
+	}
 	return h
 }
 
@@ -433,3 +437,62 @@ func (b *base64Helper) Encode() ([]byte, error) {
 func (b *base64Helper) Decode() ([]byte, error) {
 	return base64.URLEncoding.DecodeString(b.data)
 }
+
+func wildCardToRegexp(pattern string) string {
+	components := strings.Split(pattern, "*")
+	if len(components) == 1 {
+		// if len is 1, there are no *'s, return exact match pattern
+		return "^" + pattern + "$"
+	}
+	var result strings.Builder
+	for i, literal := range components {
+
+		// Replace * with .*
+		if i > 0 {
+			result.WriteString(".*")
+		}
+
+		// Quote any regular expression meta characters in the
+		// literal text.
+		result.WriteString(regexp.QuoteMeta(literal))
+	}
+	return "^" + result.String() + "$"
+}
+
+func createExcludeHeadersProcessor(excludeRegex *regexp.Regexp) headersProcessorFunc {
+	return func(headers http.Header) http.Header {
+		result := make(http.Header)
+		for k, v := range headers {
+			matched := excludeRegex.Match([]byte(k))
+			if matched {
+				continue
+			}
+			result[k] = v
+		}
+
+		return result
+	}
+}
+
+func createFullExcludeRegex(excludeHeaders string) *regexp.Regexp {
+	// comma separated list of headers to exclude from response
+	tmp := strings.Split(excludeHeaders, ",")
+
+	tmpRegexStrings := make([]string, 0)
+	for _, v := range tmp {
+		s := strings.TrimSpace(v)
+		if len(s) == 0 {
+			continue
+		}
+		pattern := wildCardToRegexp(s)
+		tmpRegexStrings = append(tmpRegexStrings, pattern)
+	}
+
+	if len(tmpRegexStrings) > 0 {
+		tmpRegexStr := strings.Join(tmpRegexStrings, "|")
+		result := regexp.MustCompile("(?i)" + "(" + tmpRegexStr + ")")
+		return result
+	}
+
+	return nil
+}
diff --git a/httpbin/httpbin.go b/httpbin/httpbin.go
index db73b66..81315fb 100644
--- a/httpbin/httpbin.go
+++ b/httpbin/httpbin.go
@@ -27,6 +27,8 @@ var DefaultDefaultParams = DefaultParams{
 	DripNumBytes: 10,
 }
 
+type headersProcessorFunc func(h http.Header) http.Header
+
 // HTTPBin contains the business logic
 type HTTPBin struct {
 	// Max size of an incoming request generated response body, in bytes
@@ -44,6 +46,7 @@ type HTTPBin struct {
 
 	// Set of hosts to which the /redirect-to endpoint will allow redirects
 	AllowedRedirectDomains map[string]struct{}
+
 	forbiddenRedirectError string
 
 	// The hostname to expose via /hostname.
@@ -51,6 +54,8 @@ type HTTPBin struct {
 
 	// The app's http handler
 	handler http.Handler
+
+	excludeHeadersProcessor headersProcessorFunc
 }
 
 // New creates a new HTTPBin instance
@@ -179,3 +184,10 @@ func (h *HTTPBin) Handler() http.Handler {
 
 	return handler
 }
+
+func (h *HTTPBin) setExcludeHeaders(excludeHeaders string) {
+	regex := createFullExcludeRegex(excludeHeaders)
+	if regex != nil {
+		h.excludeHeadersProcessor = createExcludeHeadersProcessor(regex)
+	}
+}
diff --git a/httpbin/options.go b/httpbin/options.go
index 37fb270..0fa5976 100644
--- a/httpbin/options.go
+++ b/httpbin/options.go
@@ -46,6 +46,12 @@ func WithObserver(o Observer) OptionFunc {
 	}
 }
 
+func WithExcludeHeaders(excludeHeaders string) OptionFunc {
+	return func(h *HTTPBin) {
+		h.setExcludeHeaders(excludeHeaders)
+	}
+}
+
 // WithAllowedRedirectDomains limits the domains to which the /redirect-to
 // endpoint will redirect traffic.
 func WithAllowedRedirectDomains(hosts []string) OptionFunc {


Test patch:
diff --git a/httpbin/cmd/cmd_test.go b/httpbin/cmd/cmd_test.go
index 5ce96cd..78af9a4 100644
--- a/httpbin/cmd/cmd_test.go
+++ b/httpbin/cmd/cmd_test.go
@@ -17,6 +17,8 @@ import (
 const usage = `Usage of go-httpbin:
   -allowed-redirect-domains string
     	Comma-separated list of domains the /redirect-to endpoint will allow
+  -exclude-headers string
+    	Drop platform-specific headers. Comma-separated list of headers key to drop, supporting wildcard matching.
   -host string
     	Host to listen on (default "0.0.0.0")
   -https-cert-file string
diff --git a/httpbin/handlers_test.go b/httpbin/handlers_test.go
index bef3d5f..716dd69 100644
--- a/httpbin/handlers_test.go
+++ b/httpbin/handlers_test.go
@@ -61,6 +61,7 @@ func TestMain(m *testing.M) {
 		WithMaxBodySize(maxBodySize),
 		WithMaxDuration(maxDuration),
 		WithObserver(StdLogObserver(log.New(io.Discard, "", 0))),
+		WithExcludeHeaders("x-ignore-*,x-info-this-key"),
 	)
 	srv, client = newTestServer(app)
 	defer srv.Close()
@@ -167,6 +168,28 @@ func TestGet(t *testing.T) {
 		assert.Equal(t, result.Method, "GET", "method mismatch")
 	})
 
+	t.Run("will ignore specific headers", func(t *testing.T) {
+		t.Parallel()
+
+		params := url.Values{}
+		params.Set("foo", "foo")
+		params.Add("bar", "bar1")
+		params.Add("bar", "bar2")
+
+		header := http.Header{}
+
+		header.Set("X-Ignore-Foo", "foo")
+		header.Set("X-Info-Foo", "bar")
+		header.Set("x-info-this-key", "baz")
+
+		result := doGetRequest(t, "/get", params, header)
+		assert.Equal(t, result.Args.Encode(), params.Encode(), "args mismatch")
+		assert.Equal(t, result.Method, "GET", "method mismatch")
+		assertHeaderEqual(t, &result.Headers, "X-Ignore-Foo", "")
+		assertHeaderEqual(t, &result.Headers, "x-info-this-key", "")
+		assertHeaderEqual(t, &result.Headers, "X-Info-Foo", "bar")
+	})
+
 	t.Run("only_allows_gets", func(t *testing.T) {
 		t.Parallel()
 
@@ -2920,3 +2943,11 @@ func consumeAndCloseBody(resp *http.Response) {
 	_, _ = io.Copy(io.Discard, resp.Body)
 	resp.Body.Close()
 }
+
+func assertHeaderEqual(t *testing.T, header *http.Header, key, want string) {
+	t.Helper()
+	got := header.Get(key)
+	if want != got {
+		t.Fatalf("expected header %s=%#v, got %#v", key, want, got)
+	}
+}
diff --git a/httpbin/helpers_test.go b/httpbin/helpers_test.go
index a3f7bce..71525c6 100644
--- a/httpbin/helpers_test.go
+++ b/httpbin/helpers_test.go
@@ -8,6 +8,7 @@ import (
 	"mime/multipart"
 	"net/http"
 	"net/url"
+	"regexp"
 	"testing"
 	"time"
 
@@ -276,3 +277,111 @@ func TestParseFileDoesntExist(t *testing.T) {
 		t.Fatalf("Open(nonexist): error is %T, want *PathError", err)
 	}
 }
+
+func TestWildcardHelpers(t *testing.T) {
+	tests := []struct {
+		pattern  string
+		name     string
+		input    string
+		expected bool
+	}{
+		{
+			"info-*",
+			"basic test",
+			"info-foo",
+			true,
+		},
+		{
+			"info-*",
+			"basic test case insensitive",
+			"INFO-bar",
+			true,
+		},
+		{
+			"info-*-foo",
+			"a single wildcard in the middle of the string",
+			"INFO-bar-foo",
+			true,
+		},
+		{
+			"info-*-foo",
+			"a single wildcard in the middle of the string",
+			"INFO-bar-baz",
+			false,
+		},
+		{
+			"info-*-foo-*-bar",
+			"multiple wildcards in the string",
+			"info-aaa-foo--bar",
+			true,
+		},
+		{
+			"info-*-foo-*-bar",
+			"multiple wildcards in the string",
+			"info-aaa-foo-a-bar",
+			true,
+		},
+		{
+			"info-*-foo-*-bar",
+			"multiple wildcards in the string",
+			"info-aaa-foo--bar123",
+			false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			tmpRegexStr := wildCardToRegexp(test.pattern)
+			regex := regexp.MustCompile("(?i)" + "(" + tmpRegexStr + ")")
+			matched := regex.Match([]byte(test.input))
+			assert.Equal(t, matched, test.expected, "incorrect match")
+		})
+	}
+}
+
+func TestCreateFullExcludeRegex(t *testing.T) {
+	// tolerate unused comma
+	excludeHeaders := "x-ignore-*,x-info-this-key,,"
+	regex := createFullExcludeRegex(excludeHeaders)
+	tests := []struct {
+		name     string
+		input    string
+		expected bool
+	}{
+		{
+			"basic test",
+			"x-ignore-foo",
+			true,
+		},
+		{
+			"basic test case insensitive",
+			"X-IGNORE-bar",
+			true,
+		},
+		{
+			"basic test 3",
+			"x-info-this-key",
+			true,
+		},
+		{
+			"basic test 4",
+			"foo-bar",
+			false,
+		},
+		{
+			"basic test 5",
+			"x-info-this-key-foo",
+			false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			matched := regex.Match([]byte(test.input))
+			assert.Equal(t, matched, test.expected, "incorrect match")
+		})
+	}
+
+	nilReturn := createFullExcludeRegex("")
+	assert.Equal(t, nilReturn, nil, "incorrect match")
+}