Merge pull request #33 from bloodorangeio/fix-accept-header

Only set the Accept header if explicitly set

Author: jdolitsky

client.go

@@ -82,6 +82,22 @@ func NewClient(address string, opts ...clientOption) (*Client, error) {
 	client.SetRedirectPolicy(resty.FlexibleRedirectPolicy(20))
 	client.SetTransport(createTransport(conf.InsecureSkipTLSVerify))
 
+	// TODO: disable this
+	// See https://github.com/opencontainers/distribution-spec/issues/396
+	// Restly will automatically set Accept based on Content-Type. When a user
+	// uses the "SetHeader" method, we intercept it and set the value on the
+	// context. If that context value is missing, delete it as it
+	// means Resty has automatically set the value for us (bad)
+	client.SetPreRequestHook(func(_ *resty.Client, req *http.Request) error {
+		acceptHeaderVal := req.Context().Value(contextKeyAcceptHeader)
+		if acceptHeaderVal != nil {
+			req.Header.Set("Accept", fmt.Sprintf("%s", acceptHeaderVal))
+		} else if req.Header.Get("Accept") != "" {
+			req.Header.Del("Accept")
+		}
+		return nil
+	})
+
 	return &client, nil
 }
 

client_test.go

@@ -303,4 +303,27 @@ func TestClient(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Errors executing request: %s", err)
 	}
+
+	// TODO: disable this
+	// See https://github.com/opencontainers/distribution-spec/issues/396
+	// Accept header should not be present unless explicitly set
+	req = client.NewRequest(PUT, "/a/b/c").
+		SetHeader("Content-Type", "application/vnd.oci.image.manifest.v1+json")
+	_, err = client.Do(req)
+	if err != nil {
+		t.Fatalf("Errors executing request: %s", err)
+	}
+	if a := lastCapturedRequest.Header.Get("Accept"); a != "" {
+		t.Fatalf("Expected lastCapturedAcceptHeader to be empty string, but instead got %s", a)
+	}
+	req = client.NewRequest(PUT, "/a/b/c").
+		SetHeader("Content-Type", "application/vnd.oci.image.manifest.v1+json").
+		SetHeader("Accept", "application/cha.cha.cha.v1+json")
+	_, err = client.Do(req)
+	if err != nil {
+		t.Fatalf("Errors executing request: %s", err)
+	}
+	if a := lastCapturedRequest.Header.Get("Accept"); a != "application/cha.cha.cha.v1+json" {
+		t.Fatalf("Expected lastCapturedAcceptHeader to be application/cha.cha.cha.v1+json, but instead got %s", a)
+	}
 }

request.go

@@ -1,8 +1,10 @@
 package reggie
 
 import (
+	"context"
 	"fmt"
 	"regexp"
+	"strings"
 
 	"github.com/go-resty/resty/v2"
 )
@@ -27,8 +29,12 @@ type (
 	}
 
 	requestOption func(c *requestConfig)
+
+	contextKey string
 )
 
+const contextKeyAcceptHeader contextKey = "reggie-accept"
+
 // WithName sets the namespace per a single request.
 func WithName(name string) requestOption {
 	return func(c *requestConfig) {
@@ -74,6 +80,11 @@ func (req *Request) SetBody(body interface{}) *Request {
 
 // SetHeader wraps the resty SetHeader and returns the request, allowing method chaining
 func (req *Request) SetHeader(header, content string) *Request {
+	// TODO: disable this
+	// See https://github.com/opencontainers/distribution-spec/issues/396
+	if strings.ToLower(header) == "accept" {
+		req.Request.SetContext(context.WithValue(req.Request.Context(), contextKeyAcceptHeader, content))
+	}
 	req.Request.SetHeader(header, content)
 	return req
 }