Repository: stripe/stripe-cli
Pull request #34 (2019-07-16 20:13:17)

Truncate body and headers in webhook responses
 ### Reviewers
r? @aarongreen-stripe @tomer-stripe @brandur-stripe 
cc @stripe/dev-platform

 ### Summary
Truncate webhook responses so the message stays under a certain size.

Messages sent to Stripe over the websocket connection must stay under 32 KiB, otherwise the server will reply with an unclear error message ("websocket frame too large"). This PR ensures we stay under the limit:
- the `body` field is truncated to 5000 bytes
- the `headers` map is limited to 25 entries, with keys (header names) truncated to 50 bytes and values truncated to 200 bytes

Even accounting for some overhead for JSON formatting and additional attributes, this should ensure we stay well under the limit.



Patch:
diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md
new file mode 100644
index 0000000..217ec8c
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/bug_report.md
@@ -0,0 +1,25 @@
+---
+name: Bug report
+about: Create a report to help us improve
+title: ''
+labels: bug
+assignees: ''
+
+---
+
+_The more information we have the easier it is for us to help. Feel free to remove any sections that might not apply_
+
+### Issue
+_Describe what happened and what you were trying to do_
+
+### Expected Behavior
+_Tell us what you expected to happen_
+
+### Steps to reproduce
+_What are the steps we can take to reproduce this and verify it's fixed?_
+
+### Traceback
+_Share any debug output that was given by the CLI_
+
+### Environment
+_Select one of: macOS, Linux, Windows, Other_
diff --git a/.github/ISSUE_TEMPLATE/custom.md b/.github/ISSUE_TEMPLATE/custom.md
new file mode 100644
index 0000000..857f49e
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/custom.md
@@ -0,0 +1,13 @@
+---
+name: Custom issue template
+about: Describe this issue template's purpose here.
+title: ''
+labels: ''
+assignees: ''
+
+---
+
+### Feedback
+_Give us free form feedback on the CLI. Tell us what you're building, what you're trying to do with the CLI, features you'd like to see, areas that the CLI helps, areas where it needs improvement, anything confusing you saw, etc._
+
+_Feedback can be completely freeform and stream of consciousness! If you prefer to submit private feedback, we have a form setup to do that: https://stri.pe/cli-feedback_
diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md
new file mode 100644
index 0000000..ff47dc0
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/feature_request.md
@@ -0,0 +1,17 @@
+---
+name: Feature request
+about: Suggest an idea for this project
+title: ''
+labels: enhancement
+assignees: ''
+
+---
+
+### Problem
+_What are you trying to do?_
+
+### Feature
+_What would help you accomplish your original task_
+
+### Examples
+_Any example interaction you expect_
diff --git a/README.md b/README.md
index 7815960..c62fa0a 100644
--- a/README.md
+++ b/README.md
@@ -2,6 +2,8 @@
 
 [![Build Status](https://travis-ci.com/stripe/stripe-cli.svg?token=eQWDVpt1sJR63TxbC1KA&branch=master)](https://travis-ci.com/stripe/stripe-cli)
 
+_🏗 The Stripe CLI is currently in beta! We're working on more features to make the experience great. If you have any [feedback](https://stri.pe/cli-feedback), find [issues](https://github.com/stripe/stripe-cli/issues), or would like to be involved in more active testing, please let us know!_
+
 The Stripe CLI is a command-line interface for Stripe that can:
 
 1. `login` to your Stripe account and authenticate the CLI
@@ -141,7 +143,7 @@ The webhook signing secret provided will not change between restarts to the `lis
 You can specify which events you want to listen to using `--events` with a comma-separated [list of Stripe events](https://stripe.com/docs/api/events/list).
 
 ```sh
-$ stripe listen --events=payment_intents.created,payment_intents.updated
+$ stripe listen --events=payment_intent.created,payment_intent.succeeded
 ```
 
 You may have webhook endpoints you've already configured with specific Stripe events in your Dashboard. The Stripe CLI can automatically listen to those events with the `--load-from-webhooks-api` flag, used alongside the `--forward-to` flag. This will read any endpoints configured in test mode for your account and forward associated events to the provided URL:
diff --git a/pkg/proxy/proxy.go b/pkg/proxy/proxy.go
index 953267f..7588d61 100644
--- a/pkg/proxy/proxy.go
+++ b/pkg/proxy/proxy.go
@@ -189,19 +189,26 @@ func (p *Proxy) processWebhookEvent(msg *websocket.WebhookEvent) {
 func (p *Proxy) processEndpointResponse(webhookID string, resp *http.Response) {
 	p.cfg.Log.Infof("Got response from local endpoint, status=%d", resp.StatusCode)
 
-	body, err := ioutil.ReadAll(resp.Body)
+	buf, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
 		log.Errorf("Failed to read response from endpoint, error = %v\n", err)
 		return
 	}
 
+	body := truncate(string(buf), maxBodySize, true)
+
+	idx := 0
 	headers := make(map[string]string)
 	for k, v := range resp.Header {
-		headers[k] = v[0]
+		headers[truncate(k, maxHeaderKeySize, false)] = truncate(v[0], maxHeaderValueSize, true)
+		idx++
+		if idx > maxNumHeaders {
+			break
+		}
 	}
 
 	if p.webSocketClient != nil {
-		msg := websocket.NewWebhookResponse(webhookID, resp.StatusCode, string(body), headers)
+		msg := websocket.NewWebhookResponse(webhookID, resp.StatusCode, body, headers)
 		p.webSocketClient.SendMessage(msg)
 	}
 }
@@ -238,3 +245,51 @@ func New(cfg *Config) *Proxy {
 
 	return p
 }
+
+//
+// Private constants
+//
+
+const (
+	maxBodySize        = 5000
+	maxNumHeaders      = 20
+	maxHeaderKeySize   = 50
+	maxHeaderValueSize = 200
+)
+
+//
+// Private functions
+//
+
+// truncate will truncate str to be less than or equal to maxByteLength bytes.
+// It will respect UTF8 and truncate the string at a code point boundary.
+// If ellipsis is true, we'll append "..." to the truncated string if the string
+// was in fact truncated, and if there's enough room. Note that the
+// full string returned will always be <= maxByteLength bytes long, even with ellipsis.
+func truncate(str string, maxByteLength int, ellipsis bool) string {
+	if len(str) <= maxByteLength {
+		return str
+	}
+
+	bytes := []byte(str)
+
+	if ellipsis && maxByteLength > 3 {
+		maxByteLength -= 3
+	} else {
+		ellipsis = false
+	}
+
+	for maxByteLength > 0 && maxByteLength < len(bytes) && isUTF8ContinuationByte(bytes[maxByteLength]) {
+		maxByteLength--
+	}
+
+	result := string(bytes[0:maxByteLength])
+	if ellipsis {
+		result += "..."
+	}
+	return result
+}
+
+func isUTF8ContinuationByte(b byte) bool {
+	return (b & 0xC0) == 0x80
+}
diff --git a/pkg/requests/base.go b/pkg/requests/base.go
index b2df88d..5871445 100644
--- a/pkg/requests/base.go
+++ b/pkg/requests/base.go
@@ -106,6 +106,7 @@ func (rb *Base) MakeRequest(secretKey, path string, params *RequestParameters) (
 	client := &stripe.Client{
 		BaseURL: parsedBaseURL,
 		APIKey:  secretKey,
+		Verbose: rb.showHeaders,
 	}
 
 	data, err := rb.buildDataForRequest(params)
@@ -131,10 +132,6 @@ func (rb *Base) MakeRequest(secretKey, path string, params *RequestParameters) (
 			return []byte{}, err
 		}
 
-		if rb.showHeaders {
-			fmt.Println(rb.formatHeaders(resp))
-		}
-
 		result := ansi.ColorizeJSON(string(body), os.Stdout)
 		fmt.Println(result)
 	}
@@ -212,16 +209,6 @@ func encode(keys []string, values []string) string {
 	return buf.String()
 }
 
-func (rb *Base) formatHeaders(response *http.Response) string {
-	var allHeaders []string
-	for name, headers := range response.Header {
-		for _, h := range headers {
-			allHeaders = append(allHeaders, fmt.Sprintf("< %v: %v", name, h))
-		}
-	}
-	return strings.Join(allHeaders, "\n") + "\n"
-}
-
 func (rb *Base) setIdempotencyHeader(request *http.Request, params *RequestParameters) {
 	if params.idempotency != "" {
 		request.Header.Set("Idempotency-Key", params.idempotency)
diff --git a/pkg/stripe/client.go b/pkg/stripe/client.go
index bdc64fe..5af7cb0 100644
--- a/pkg/stripe/client.go
+++ b/pkg/stripe/client.go
@@ -29,6 +29,10 @@ type Client struct {
 	// empty, the `Authorization` header will be omitted.
 	APIKey string
 
+	// When this is enabled, request and response headers will be printed to
+	// stdout.
+	Verbose bool
+
 	// Cached HTTP client, lazily created the first time the Client is used to
 	// send a request.
 	httpClient *http.Client
@@ -68,7 +72,7 @@ func (c *Client) PerformRequest(method, path string, params string, configure fu
 	}
 
 	if c.httpClient == nil {
-		c.httpClient = newHTTPClient(os.Getenv("STRIPE_CLI_UNIX_SOCKET"))
+		c.httpClient = newHTTPClient(c.Verbose, os.Getenv("STRIPE_CLI_UNIX_SOCKET"))
 	}
 
 	resp, err := c.httpClient.Do(req)
@@ -79,7 +83,7 @@ func (c *Client) PerformRequest(method, path string, params string, configure fu
 	return resp, nil
 }
 
-func newHTTPClient(unixSocket string) *http.Client {
+func newHTTPClient(verbose bool, unixSocket string) *http.Client {
 	var httpTransport *http.Transport
 	if unixSocket != "" {
 		dialFunc := func(network, addr string) (net.Conn, error) {
@@ -105,7 +109,14 @@ func newHTTPClient(unixSocket string) *http.Client {
 			TLSHandshakeTimeout: 10 * time.Second,
 		}
 	}
-	return &http.Client{
+
+	tr := &verboseTransport{
 		Transport: httpTransport,
+		Verbose:   verbose,
+		Out:       os.Stderr,
+	}
+
+	return &http.Client{
+		Transport: tr,
 	}
 }
diff --git a/pkg/stripe/verbosetransport.go b/pkg/stripe/verbosetransport.go
new file mode 100644
index 0000000..22b757a
--- /dev/null
+++ b/pkg/stripe/verbosetransport.go
@@ -0,0 +1,81 @@
+package stripe
+
+import (
+	"fmt"
+	"io"
+	"net/http"
+	"regexp"
+	"strings"
+
+	"github.com/stripe/stripe-cli/pkg/ansi"
+)
+
+// inspectHeaders is the whitelist of headers that will be printed.
+var inspectHeaders = []string{
+	"Authorization",
+	"Content-Type",
+	"Date",
+	"Idempotency-Key",
+	"Idempotency-Replayed",
+	"Request-Id",
+	"Stripe-Account",
+	"Stripe-Version",
+}
+
+type verboseTransport struct {
+	Transport *http.Transport
+	Verbose   bool
+	Out       io.Writer
+}
+
+func (t *verboseTransport) RoundTrip(req *http.Request) (resp *http.Response, err error) {
+	if t.Verbose {
+		t.dumpRequest(req)
+	}
+
+	resp, err = t.Transport.RoundTrip(req)
+
+	if err == nil && t.Verbose {
+		t.dumpResponse(resp)
+	}
+
+	return
+}
+
+func (t *verboseTransport) dumpRequest(req *http.Request) {
+	info := fmt.Sprintf("> %s %s://%s%s", req.Method, req.URL.Scheme, req.URL.Host, req.URL.RequestURI())
+	t.verbosePrintln(info)
+	t.dumpHeaders(req.Header, ">")
+}
+
+func (t *verboseTransport) dumpResponse(resp *http.Response) {
+	info := fmt.Sprintf("< HTTP %d", resp.StatusCode)
+	t.verbosePrintln(info)
+	t.dumpHeaders(resp.Header, "<")
+}
+
+func (t *verboseTransport) dumpHeaders(header http.Header, indent string) {
+	for _, listed := range inspectHeaders {
+		for name, vv := range header {
+			if !strings.EqualFold(name, listed) {
+				continue
+			}
+			for _, v := range vv {
+				if v != "" {
+					r := regexp.MustCompile("(?i)^(basic|bearer) (.+)")
+					if r.MatchString(v) {
+						v = r.ReplaceAllString(v, "$1 [REDACTED]")
+					}
+
+					info := fmt.Sprintf("%s %s: %s", indent, name, v)
+					t.verbosePrintln(info)
+				}
+			}
+		}
+	}
+}
+
+func (t *verboseTransport) verbosePrintln(msg string) {
+	color := ansi.Color(t.Out)
+	fmt.Fprintln(t.Out, color.Cyan(msg))
+}


Test patch:
diff --git a/pkg/proxy/proxy_test.go b/pkg/proxy/proxy_test.go
index 59840ac..d4454d2 100644
--- a/pkg/proxy/proxy_test.go
+++ b/pkg/proxy/proxy_test.go
@@ -31,3 +31,13 @@ func TestFilterWebhookEvent(t *testing.T) {
 	assert.True(t, proxyUseLatest.filterWebhookEvent(evtDefault))
 	assert.False(t, proxyUseLatest.filterWebhookEvent(evtLatest))
 }
+
+func TestTruncate(t *testing.T) {
+	assert.Equal(t, "Hello, World", truncate("Hello, World", 12, false))
+	assert.Equal(t, "Hello, Worl", truncate("Hello, World", 11, false))
+	assert.Equal(t, "Hello, W...", truncate("Hello, World", 11, true))
+
+	assert.Equal(t, "Hello, 世界", truncate("Hello, 世界", 13, false))
+	assert.Equal(t, "Hello, 世", truncate("Hello, 世界", 12, false))
+	assert.Equal(t, "Hello, ...", truncate("Hello, 世界", 12, true))
+}
diff --git a/pkg/requests/base_test.go b/pkg/requests/base_test.go
index f21f113..fd971e1 100644
--- a/pkg/requests/base_test.go
+++ b/pkg/requests/base_test.go
@@ -110,31 +110,6 @@ func TestMakeRequest(t *testing.T) {
 	assert.Nil(t, err)
 }
 
-func TestFormatHeaders(t *testing.T) {
-	rb := Base{}
-
-	resp := &http.Response{
-		Header: make(http.Header, 0),
-	}
-
-	emptyHeaders := rb.formatHeaders(resp)
-	emptyExpected := "\n"
-
-	assert.Equal(t, emptyExpected, emptyHeaders)
-
-	resp.Header.Add("header-one", "header-one-value")
-	resp.Header.Add("header-two", "header-two-value")
-	resp.Header.Add("header-three", "header-three-value")
-
-	headers := rb.formatHeaders(resp)
-
-	// Since Headers are stored in a map, the order is not deterministic
-	// and we must check for contains each header vs. direct string comparison
-	assert.Contains(t, headers, "< Header-One: header-one-value\n")
-	assert.Contains(t, headers, "< Header-Two: header-two-value\n")
-	assert.Contains(t, headers, "< Header-Three: header-three-value\n")
-}
-
 func TestGetUserConfirmationRequired(t *testing.T) {
 	reader := bufio.NewReader(strings.NewReader("yes\n"))
 
diff --git a/pkg/stripe/verbosetransport_test.go b/pkg/stripe/verbosetransport_test.go
new file mode 100644
index 0000000..403c22d
--- /dev/null
+++ b/pkg/stripe/verbosetransport_test.go
@@ -0,0 +1,41 @@
+package stripe
+
+import (
+	"bytes"
+	"net/http"
+	"net/http/httptest"
+	"regexp"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestVerboseTransport_Verbose(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Request-Id", "req_123")
+		w.Header().Set("Non-Whitelisted-Header", "foo")
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer ts.Close()
+
+	var b bytes.Buffer
+	httpTransport := &http.Transport{}
+	tr := &verboseTransport{
+		Transport: httpTransport,
+		Verbose:   true,
+		Out:       &b,
+	}
+	client := &http.Client{Transport: tr}
+	req, _ := http.NewRequest("POST", ts.URL+"/test", nil)
+	req.Header.Set("Authorization", "Bearer token")
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	client.Do(req)
+
+	out := b.String()
+	assert.Regexp(t, regexp.MustCompile("> POST http://(.+)/test\n"), out)
+	assert.Contains(t, out, "> Authorization: Bearer [REDACTED]\n")
+	assert.Contains(t, out, "> Content-Type: application/x-www-form-urlencoded\n")
+	assert.Contains(t, out, "< HTTP 200\n")
+	assert.Contains(t, out, "< Request-Id: req_123\n")
+	assert.NotContains(t, out, "Non-Whitelisted-Header")
+}