Repository: googlemaps/google-maps-services-go (go)

Issue:
nil pointer dereference on failed binary request
#### Environment details

1. Places API / Place Photos
2. macOS 10.15
3. v1.2.3

#### Steps to reproduce

  1. Make an API which uses `Client.getBinary`, e.g. `Client.PlacePhoto`
  2. Have the request fail, e.g. through context cancellation
  3. See error `panic: runtime error: invalid memory address or nil pointer dereference`

#### Stack trace
```
googlemaps.github.io/maps.(*Client).getBinary(0xc000379b80, 0x183e340, 0xc000a76f00, 0x1cb5fe0, 0x182b1a0, 0xc0002e5100, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/mgmcintyre/go/pkg/mod/googlemaps.github.io/maps@v1.2.3/client.go:312 +0x10c
googlemaps.github.io/maps.(*Client).PlacePhoto(0xc000379b80, 0x183e340, 0xc000a76f00, 0xc0002e5100, 0x0, 0x1, 0x4, 0x0, 0x0, 0x0)
	/Users/mgmcintyre/go/pkg/mod/googlemaps.github.io/maps@v1.2.3/places.go:814 +0x9c
```

Pull request:
fix: handles case where binary request does not return a response
Fixes #250 🦕

Mirrors error handling approach from `getJSON` / `postJSON` in `getBinary`.


Patch:
diff --git a/client.go b/client.go
index b19091e..70b491d 100644
--- a/client.go
+++ b/client.go
@@ -50,8 +50,14 @@ type ClientOption func(*Client) error
 
 var defaultRequestsPerSecond = 50
 
+type contextKey string
+func (c contextKey) String() string {
+	return "maps " + string(c)
+}
+
 const (
 	ExperienceIdHeaderName = "X-GOOG-MAPS-EXPERIENCE-ID"
+	contextExperienceId    = contextKey("EXP-IDS")
 )
 
 // NewClient constructs a new Client which can make requests to the Google Maps
@@ -118,6 +124,23 @@ func WithAPIKeyAndSignature(apiKey, signature string) ClientOption {
 	}
 }
 
+// ExperienceIdContext injects the experienceIds in the context, from where they will be pull out
+// in the post/get handlers. Useful if a customer uses one client instance per different experiences calls
+func ExperienceIdContext(ctx context.Context, experienceIds ...string) context.Context {
+	if ctx != nil {
+		return context.WithValue(ctx, contextExperienceId, experienceIds)
+	}
+	return ctx
+}
+
+// ExperienceIdFromContext returns experienceIds from context if presented
+func ExperienceIdFromContext(ctx context.Context) []string {
+	if experiencesId := ctx.Value(contextExperienceId); experiencesId != nil {
+		return experiencesId.([]string)
+	}
+	return nil
+}
+
 // WithBaseURL configures a Maps API client with a custom base url
 func WithBaseURL(baseURL string) ClientOption {
 	return func(c *Client) error {
@@ -205,7 +228,7 @@ func (c *Client) get(ctx context.Context, config *apiConfig, apiReq apiRequest)
 		return nil, err
 	}
 
-	c.setExperienceIdHeader(req)
+	c.setExperienceIdHeader(ctx, req)
 
 	q, err := c.generateAuthQuery(config.path, apiReq.params(), config.acceptsClientID, config.acceptsSignature)
 	if err != nil {
@@ -235,7 +258,7 @@ func (c *Client) post(ctx context.Context, config *apiConfig, apiReq interface{}
 	}
 	req.Header.Set("Content-Type", "application/json")
 
-	c.setExperienceIdHeader(req)
+	c.setExperienceIdHeader(ctx, req)
 
 	q, err := c.generateAuthQuery(config.path, url.Values{}, config.acceptsClientID, config.acceptsSignature)
 	if err != nil {
@@ -294,9 +317,18 @@ func (c *Client) clearExperienceId() {
 	c.experienceId = nil
 }
 
-func (c *Client) setExperienceIdHeader(req *http.Request) {
+func (c *Client) setExperienceIdHeader(ctx context.Context, req *http.Request) {
+	var ids []string
 	if len(c.experienceId) > 0 {
-		req.Header.Set(ExperienceIdHeaderName, strings.Join(c.experienceId, ","))
+		ids = append(ids, c.experienceId...)
+	}
+	if experiencesId := ExperienceIdFromContext(ctx); experiencesId != nil {
+		for _, v := range experiencesId {
+			ids = append(ids, v)
+		}
+	}
+	if len(ids) != 0 {
+		req.Header.Set(ExperienceIdHeaderName, strings.Join(ids, ","))
 	}
 }
 
@@ -309,11 +341,12 @@ type binaryResponse struct {
 func (c *Client) getBinary(ctx context.Context, config *apiConfig, apiReq apiRequest) (binaryResponse, error) {
 	requestMetrics := c.metricReporter.NewRequest(config.path)
 	httpResp, err := c.get(ctx, config, apiReq)
-	requestMetrics.EndRequest(ctx, err, httpResp, httpResp.Header.Get("x-goog-maps-metro-area"))
 	if err != nil {
+		requestMetrics.EndRequest(ctx, err, httpResp, "")
 		return binaryResponse{}, err
 	}
 
+	requestMetrics.EndRequest(ctx, err, httpResp, httpResp.Header.Get("x-goog-maps-metro-area"))
 	return binaryResponse{httpResp.StatusCode, httpResp.Header.Get("Content-Type"), httpResp.Body}, nil
 }
 


Test patch:
diff --git a/client_test.go b/client_test.go
index 30c9a8d..98f99a0 100644
--- a/client_test.go
+++ b/client_test.go
@@ -15,6 +15,7 @@
 package maps
 
 import (
+	"context"
 	"net/http"
 	"strings"
 	"testing"
@@ -69,20 +70,45 @@ func TestClientSetExperienceIdHeader(t *testing.T) {
 	// slice has two elements
 	c.experienceId = ids
 	req, _ := http.NewRequest("GET", "/", nil)
-	c.setExperienceIdHeader(req)
-	assert.Equal(t, req.Header.Get(ExperienceIdHeaderName), strings.Join(ids, ","))
+	c.setExperienceIdHeader(context.Background(), req)
+	assert.Equal(t, strings.Join(ids, ","), req.Header.Get(ExperienceIdHeaderName))
 
 	// slice is nil
 	c.experienceId = nil
 	req, _ = http.NewRequest("GET", "/", nil)
-	c.setExperienceIdHeader(req)
-	assert.Equal(t, req.Header.Get(ExperienceIdHeaderName), "")
+	c.setExperienceIdHeader(context.Background(), req)
+	assert.Equal(t, "", req.Header.Get(ExperienceIdHeaderName))
 
 	// slice is empty
 	c.experienceId = []string{}
 	req, _ = http.NewRequest("GET", "/", nil)
-	c.setExperienceIdHeader(req)
-	assert.Equal(t, req.Header.Get(ExperienceIdHeaderName), "")
+	c.setExperienceIdHeader(context.Background(), req)
+	assert.Equal(t, "", req.Header.Get(ExperienceIdHeaderName))
+
+	var ctx context.Context
+	// context has one element
+	c.experienceId = []string{}
+	ctx = context.Background()
+	ctx = ExperienceIdContext(ctx, "foo")
+	req, _ = http.NewRequest("GET", "/", nil)
+	c.setExperienceIdHeader(ctx, req)
+	assert.Equal(t, "foo", req.Header.Get(ExperienceIdHeaderName))
+
+	// context has two elements
+	c.experienceId = []string{}
+	ctx = context.Background()
+	ctx = ExperienceIdContext(ctx, ids...)
+	req, _ = http.NewRequest("GET", "/", nil)
+	c.setExperienceIdHeader(ctx, req)
+	assert.Equal(t, strings.Join(ids, ","), req.Header.Get(ExperienceIdHeaderName))
+
+	// context has two elements and client has two elements
+	c.experienceId = ids
+	ctx = context.Background()
+	ctx = ExperienceIdContext(ctx, ids...)
+	req, _ = http.NewRequest("GET", "/", nil)
+	c.setExperienceIdHeader(ctx, req)
+	assert.Equal(t, strings.Join(ids, ",") + "," + strings.Join(ids, ","), req.Header.Get(ExperienceIdHeaderName))
 }
 
 func TestClientExperienceIdSample(t *testing.T) {