From 876b1c6ee618a9f8fa31ded3b27708d44b3153af Mon Sep 17 00:00:00 2001 From: Ross Light Date: Tue, 2 Jan 2018 16:32:36 -0800 Subject: [PATCH] internal: remove RegisterContextClientFunc This function added a totally unused error path, since the only call site is for App Engine, which cannot produce an error. Change-Id: I86277ab4ff96e7bd140c53c5a114a338716668e3 Reviewed-on: https://go-review.googlesource.com/85935 Reviewed-by: Brad Fitzpatrick --- client_appengine.go | 25 -------------------- internal/client_appengine.go | 13 ++++++++++ internal/token.go | 6 +---- internal/transport.go | 46 +++++------------------------------- internal/transport_test.go | 38 ----------------------------- oauth2.go | 8 ++----- 6 files changed, 22 insertions(+), 114 deletions(-) delete mode 100644 client_appengine.go create mode 100644 internal/client_appengine.go delete mode 100644 internal/transport_test.go diff --git a/client_appengine.go b/client_appengine.go deleted file mode 100644 index 8962c49..0000000 --- a/client_appengine.go +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2014 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// +build appengine - -// App Engine hooks. - -package oauth2 - -import ( - "net/http" - - "golang.org/x/net/context" - "golang.org/x/oauth2/internal" - "google.golang.org/appengine/urlfetch" -) - -func init() { - internal.RegisterContextClientFunc(contextClientAppEngine) -} - -func contextClientAppEngine(ctx context.Context) (*http.Client, error) { - return urlfetch.Client(ctx), nil -} diff --git a/internal/client_appengine.go b/internal/client_appengine.go new file mode 100644 index 0000000..7434871 --- /dev/null +++ b/internal/client_appengine.go @@ -0,0 +1,13 @@ +// Copyright 2018 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// +build appengine + +package internal + +import "google.golang.org/appengine/urlfetch" + +func init() { + appengineClientHook = urlfetch.Client +} diff --git a/internal/token.go b/internal/token.go index 558ce95..881fbef 100644 --- a/internal/token.go +++ b/internal/token.go @@ -170,10 +170,6 @@ func providerAuthHeaderWorks(tokenURL string) bool { } func RetrieveToken(ctx context.Context, clientID, clientSecret, tokenURL string, v url.Values) (*Token, error) { - hc, err := ContextClient(ctx) - if err != nil { - return nil, err - } bustedAuth := !providerAuthHeaderWorks(tokenURL) if bustedAuth { if clientID != "" { @@ -191,7 +187,7 @@ func RetrieveToken(ctx context.Context, clientID, clientSecret, tokenURL string, if !bustedAuth { req.SetBasicAuth(url.QueryEscape(clientID), url.QueryEscape(clientSecret)) } - r, err := ctxhttp.Do(ctx, hc, req) + r, err := ctxhttp.Do(ctx, ContextClient(ctx), req) if err != nil { return nil, err } diff --git a/internal/transport.go b/internal/transport.go index 783bd98..d16f9ae 100644 --- a/internal/transport.go +++ b/internal/transport.go @@ -19,50 +19,16 @@ var HTTPClient ContextKey // because nobody else can create a ContextKey, being unexported. type ContextKey struct{} -// ContextClientFunc is a func which tries to return an *http.Client -// given a Context value. If it returns an error, the search stops -// with that error. If it returns (nil, nil), the search continues -// down the list of registered funcs. -type ContextClientFunc func(context.Context) (*http.Client, error) +var appengineClientHook func(context.Context) *http.Client -var contextClientFuncs []ContextClientFunc - -func RegisterContextClientFunc(fn ContextClientFunc) { - contextClientFuncs = append(contextClientFuncs, fn) -} - -func ContextClient(ctx context.Context) (*http.Client, error) { +func ContextClient(ctx context.Context) *http.Client { if ctx != nil { if hc, ok := ctx.Value(HTTPClient).(*http.Client); ok { - return hc, nil + return hc } } - for _, fn := range contextClientFuncs { - c, err := fn(ctx) - if err != nil { - return nil, err - } - if c != nil { - return c, nil - } + if appengineClientHook != nil { + return appengineClientHook(ctx) } - return http.DefaultClient, nil -} - -func ContextTransport(ctx context.Context) http.RoundTripper { - hc, err := ContextClient(ctx) - // This is a rare error case (somebody using nil on App Engine). - if err != nil { - return ErrorTransport{err} - } - return hc.Transport -} - -// ErrorTransport returns the specified error on RoundTrip. -// This RoundTripper should be used in rare error cases where -// error handling can be postponed to response handling time. -type ErrorTransport struct{ Err error } - -func (t ErrorTransport) RoundTrip(*http.Request) (*http.Response, error) { - return nil, t.Err + return http.DefaultClient } diff --git a/internal/transport_test.go b/internal/transport_test.go deleted file mode 100644 index 8772ec5..0000000 --- a/internal/transport_test.go +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2015 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package internal - -import ( - "net/http" - "testing" - - "golang.org/x/net/context" -) - -func TestContextClient(t *testing.T) { - rc := &http.Client{} - RegisterContextClientFunc(func(context.Context) (*http.Client, error) { - return rc, nil - }) - - c := &http.Client{} - ctx := context.WithValue(context.Background(), HTTPClient, c) - - hc, err := ContextClient(ctx) - if err != nil { - t.Fatalf("want valid client; got err = %v", err) - } - if hc != c { - t.Fatalf("want context client = %p; got = %p", c, hc) - } - - hc, err = ContextClient(context.TODO()) - if err != nil { - t.Fatalf("want valid client; got err = %v", err) - } - if hc != rc { - t.Fatalf("want registered client = %p; got = %p", c, hc) - } -} diff --git a/oauth2.go b/oauth2.go index 6266950..7234e00 100644 --- a/oauth2.go +++ b/oauth2.go @@ -313,15 +313,11 @@ var HTTPClient internal.ContextKey // packages. func NewClient(ctx context.Context, src TokenSource) *http.Client { if src == nil { - c, err := internal.ContextClient(ctx) - if err != nil { - return &http.Client{Transport: internal.ErrorTransport{Err: err}} - } - return c + return internal.ContextClient(ctx) } return &http.Client{ Transport: &Transport{ - Base: internal.ContextTransport(ctx), + Base: internal.ContextClient(ctx).Transport, Source: ReuseTokenSource(nil, src), }, }