From db107e3e2cd639f6fb8a5a2ed8d84bcb4203818f Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Thu, 14 May 2015 07:12:54 -0700 Subject: [PATCH 1/3] registry: Refactor requestfactory to use http.RoundTrippers This patch removes the need for requestFactories and decorators by implementing http.RoundTripper transports instead. It refactors some challenging-to-read code. NewSession now takes an *http.Client that can already have a custom Transport, it will add its own auth transport by wrapping it. The idea is that callers of http.Client should not bother setting custom headers for every handler but instead it should be transparent to the callers of a same context. This patch is needed for future refactorings of registry, namely refactoring of the v1 client code. Signed-off-by: Tibor Vass --- requestdecorator/requestdecorator.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/requestdecorator/requestdecorator.go b/requestdecorator/requestdecorator.go index c236e3f..079e38b 100644 --- a/requestdecorator/requestdecorator.go +++ b/requestdecorator/requestdecorator.go @@ -47,7 +47,7 @@ func (vi *UAVersionInfo) isValid() bool { // "product/version", where the "product" is get from the name field, while // version is get from the version field. Several pieces of verson information // will be concatinated and separated by space. -func appendVersions(base string, versions ...UAVersionInfo) string { +func AppendVersions(base string, versions ...UAVersionInfo) string { if len(versions) == 0 { return base } @@ -87,7 +87,7 @@ func (h *UserAgentDecorator) ChangeRequest(req *http.Request) (*http.Request, er return req, ErrNilRequest } - userAgent := appendVersions(req.UserAgent(), h.Versions...) + userAgent := AppendVersions(req.UserAgent(), h.Versions...) if len(userAgent) > 0 { req.Header.Set("User-Agent", userAgent) } From 4b4923487d0bfdea16681d68e3329d153883c663 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Fri, 15 May 2015 15:03:08 -0700 Subject: [PATCH 2/3] requestdecorator: repurpose the package and rename to useragent Signed-off-by: Tibor Vass --- requestdecorator/README.md | 2 - requestdecorator/requestdecorator.go | 172 ----------------- requestdecorator/requestdecorator_test.go | 222 ---------------------- useragent/README.md | 1 + useragent/useragent.go | 60 ++++++ useragent/useragent_test.go | 31 +++ 6 files changed, 92 insertions(+), 396 deletions(-) delete mode 100644 requestdecorator/README.md delete mode 100644 requestdecorator/requestdecorator.go delete mode 100644 requestdecorator/requestdecorator_test.go create mode 100644 useragent/README.md create mode 100644 useragent/useragent.go create mode 100644 useragent/useragent_test.go diff --git a/requestdecorator/README.md b/requestdecorator/README.md deleted file mode 100644 index 76f8ca7..0000000 --- a/requestdecorator/README.md +++ /dev/null @@ -1,2 +0,0 @@ -This package provides helper functions for decorating a request with user agent -versions, auth, meta headers. diff --git a/requestdecorator/requestdecorator.go b/requestdecorator/requestdecorator.go deleted file mode 100644 index 079e38b..0000000 --- a/requestdecorator/requestdecorator.go +++ /dev/null @@ -1,172 +0,0 @@ -// Package requestdecorator provides helper functions to decorate a request with -// user agent versions, auth, meta headers. -package requestdecorator - -import ( - "errors" - "io" - "net/http" - "strings" - - "github.com/Sirupsen/logrus" -) - -var ( - ErrNilRequest = errors.New("request cannot be nil") -) - -// UAVersionInfo is used to model UserAgent versions. -type UAVersionInfo struct { - Name string - Version string -} - -func NewUAVersionInfo(name, version string) UAVersionInfo { - return UAVersionInfo{ - Name: name, - Version: version, - } -} - -func (vi *UAVersionInfo) isValid() bool { - const stopChars = " \t\r\n/" - name := vi.Name - vers := vi.Version - if len(name) == 0 || strings.ContainsAny(name, stopChars) { - return false - } - if len(vers) == 0 || strings.ContainsAny(vers, stopChars) { - return false - } - return true -} - -// Convert versions to a string and append the string to the string base. -// -// Each UAVersionInfo will be converted to a string in the format of -// "product/version", where the "product" is get from the name field, while -// version is get from the version field. Several pieces of verson information -// will be concatinated and separated by space. -func AppendVersions(base string, versions ...UAVersionInfo) string { - if len(versions) == 0 { - return base - } - - verstrs := make([]string, 0, 1+len(versions)) - if len(base) > 0 { - verstrs = append(verstrs, base) - } - - for _, v := range versions { - if !v.isValid() { - continue - } - verstrs = append(verstrs, v.Name+"/"+v.Version) - } - return strings.Join(verstrs, " ") -} - -// Decorator is used to change an instance of -// http.Request. It could be used to add more header fields, -// change body, etc. -type Decorator interface { - // ChangeRequest() changes the request accordingly. - // The changed request will be returned or err will be non-nil - // if an error occur. - ChangeRequest(req *http.Request) (newReq *http.Request, err error) -} - -// UserAgentDecorator appends the product/version to the user agent field -// of a request. -type UserAgentDecorator struct { - Versions []UAVersionInfo -} - -func (h *UserAgentDecorator) ChangeRequest(req *http.Request) (*http.Request, error) { - if req == nil { - return req, ErrNilRequest - } - - userAgent := AppendVersions(req.UserAgent(), h.Versions...) - if len(userAgent) > 0 { - req.Header.Set("User-Agent", userAgent) - } - return req, nil -} - -type MetaHeadersDecorator struct { - Headers map[string][]string -} - -func (h *MetaHeadersDecorator) ChangeRequest(req *http.Request) (*http.Request, error) { - if h.Headers == nil { - return req, ErrNilRequest - } - for k, v := range h.Headers { - req.Header[k] = v - } - return req, nil -} - -type AuthDecorator struct { - login string - password string -} - -func NewAuthDecorator(login, password string) Decorator { - return &AuthDecorator{ - login: login, - password: password, - } -} - -func (self *AuthDecorator) ChangeRequest(req *http.Request) (*http.Request, error) { - if req == nil { - return req, ErrNilRequest - } - req.SetBasicAuth(self.login, self.password) - return req, nil -} - -// RequestFactory creates an HTTP request -// and applies a list of decorators on the request. -type RequestFactory struct { - decorators []Decorator -} - -func NewRequestFactory(d ...Decorator) *RequestFactory { - return &RequestFactory{ - decorators: d, - } -} - -func (f *RequestFactory) AddDecorator(d ...Decorator) { - f.decorators = append(f.decorators, d...) -} - -func (f *RequestFactory) GetDecorators() []Decorator { - return f.decorators -} - -// NewRequest() creates a new *http.Request, -// applies all decorators in the Factory on the request, -// then applies decorators provided by d on the request. -func (h *RequestFactory) NewRequest(method, urlStr string, body io.Reader, d ...Decorator) (*http.Request, error) { - req, err := http.NewRequest(method, urlStr, body) - if err != nil { - return nil, err - } - - // By default, a nil factory should work. - if h == nil { - return req, nil - } - for _, dec := range h.decorators { - req, _ = dec.ChangeRequest(req) - } - for _, dec := range d { - req, _ = dec.ChangeRequest(req) - } - logrus.Debugf("%v -- HEADERS: %v", req.URL, req.Header) - return req, err -} diff --git a/requestdecorator/requestdecorator_test.go b/requestdecorator/requestdecorator_test.go deleted file mode 100644 index ed61135..0000000 --- a/requestdecorator/requestdecorator_test.go +++ /dev/null @@ -1,222 +0,0 @@ -package requestdecorator - -import ( - "net/http" - "strings" - "testing" -) - -func TestUAVersionInfo(t *testing.T) { - uavi := NewUAVersionInfo("foo", "bar") - if !uavi.isValid() { - t.Fatalf("UAVersionInfo should be valid") - } - uavi = NewUAVersionInfo("", "bar") - if uavi.isValid() { - t.Fatalf("Expected UAVersionInfo to be invalid") - } - uavi = NewUAVersionInfo("foo", "") - if uavi.isValid() { - t.Fatalf("Expected UAVersionInfo to be invalid") - } -} - -func TestUserAgentDecorator(t *testing.T) { - httpVersion := make([]UAVersionInfo, 2) - httpVersion = append(httpVersion, NewUAVersionInfo("testname", "testversion")) - httpVersion = append(httpVersion, NewUAVersionInfo("name", "version")) - uad := &UserAgentDecorator{ - Versions: httpVersion, - } - - req, err := http.NewRequest("GET", "/something", strings.NewReader("test")) - if err != nil { - t.Fatal(err) - } - reqDecorated, err := uad.ChangeRequest(req) - if err != nil { - t.Fatal(err) - } - - if reqDecorated.Header.Get("User-Agent") != "testname/testversion name/version" { - t.Fatalf("Request should have User-Agent 'testname/testversion name/version'") - } -} - -func TestUserAgentDecoratorErr(t *testing.T) { - httpVersion := make([]UAVersionInfo, 0) - uad := &UserAgentDecorator{ - Versions: httpVersion, - } - - var req *http.Request - _, err := uad.ChangeRequest(req) - if err == nil { - t.Fatalf("Expected to get ErrNilRequest instead no error was returned") - } -} - -func TestMetaHeadersDecorator(t *testing.T) { - var headers = map[string][]string{ - "key1": {"value1"}, - "key2": {"value2"}, - } - mhd := &MetaHeadersDecorator{ - Headers: headers, - } - - req, err := http.NewRequest("GET", "/something", strings.NewReader("test")) - if err != nil { - t.Fatal(err) - } - reqDecorated, err := mhd.ChangeRequest(req) - if err != nil { - t.Fatal(err) - } - - v, ok := reqDecorated.Header["key1"] - if !ok { - t.Fatalf("Expected to have header key1") - } - if v[0] != "value1" { - t.Fatalf("Expected value for key1 isn't value1") - } - - v, ok = reqDecorated.Header["key2"] - if !ok { - t.Fatalf("Expected to have header key2") - } - if v[0] != "value2" { - t.Fatalf("Expected value for key2 isn't value2") - } -} - -func TestMetaHeadersDecoratorErr(t *testing.T) { - mhd := &MetaHeadersDecorator{} - - var req *http.Request - _, err := mhd.ChangeRequest(req) - if err == nil { - t.Fatalf("Expected to get ErrNilRequest instead no error was returned") - } -} - -func TestAuthDecorator(t *testing.T) { - ad := NewAuthDecorator("test", "password") - - req, err := http.NewRequest("GET", "/something", strings.NewReader("test")) - if err != nil { - t.Fatal(err) - } - reqDecorated, err := ad.ChangeRequest(req) - if err != nil { - t.Fatal(err) - } - - username, password, ok := reqDecorated.BasicAuth() - if !ok { - t.Fatalf("Cannot retrieve basic auth info from request") - } - if username != "test" { - t.Fatalf("Expected username to be test, got %s", username) - } - if password != "password" { - t.Fatalf("Expected password to be password, got %s", password) - } -} - -func TestAuthDecoratorErr(t *testing.T) { - ad := &AuthDecorator{} - - var req *http.Request - _, err := ad.ChangeRequest(req) - if err == nil { - t.Fatalf("Expected to get ErrNilRequest instead no error was returned") - } -} - -func TestRequestFactory(t *testing.T) { - ad := NewAuthDecorator("test", "password") - httpVersion := make([]UAVersionInfo, 2) - httpVersion = append(httpVersion, NewUAVersionInfo("testname", "testversion")) - httpVersion = append(httpVersion, NewUAVersionInfo("name", "version")) - uad := &UserAgentDecorator{ - Versions: httpVersion, - } - - requestFactory := NewRequestFactory(ad, uad) - - if l := len(requestFactory.GetDecorators()); l != 2 { - t.Fatalf("Expected to have two decorators, got %d", l) - } - - req, err := requestFactory.NewRequest("GET", "/test", strings.NewReader("test")) - if err != nil { - t.Fatal(err) - } - - username, password, ok := req.BasicAuth() - if !ok { - t.Fatalf("Cannot retrieve basic auth info from request") - } - if username != "test" { - t.Fatalf("Expected username to be test, got %s", username) - } - if password != "password" { - t.Fatalf("Expected password to be password, got %s", password) - } - if req.Header.Get("User-Agent") != "testname/testversion name/version" { - t.Fatalf("Request should have User-Agent 'testname/testversion name/version'") - } -} - -func TestRequestFactoryNewRequestWithDecorators(t *testing.T) { - ad := NewAuthDecorator("test", "password") - - requestFactory := NewRequestFactory(ad) - - if l := len(requestFactory.GetDecorators()); l != 1 { - t.Fatalf("Expected to have one decorators, got %d", l) - } - - ad2 := NewAuthDecorator("test2", "password2") - - req, err := requestFactory.NewRequest("GET", "/test", strings.NewReader("test"), ad2) - if err != nil { - t.Fatal(err) - } - - username, password, ok := req.BasicAuth() - if !ok { - t.Fatalf("Cannot retrieve basic auth info from request") - } - if username != "test2" { - t.Fatalf("Expected username to be test, got %s", username) - } - if password != "password2" { - t.Fatalf("Expected password to be password, got %s", password) - } -} - -func TestRequestFactoryAddDecorator(t *testing.T) { - requestFactory := NewRequestFactory() - - if l := len(requestFactory.GetDecorators()); l != 0 { - t.Fatalf("Expected to have zero decorators, got %d", l) - } - - ad := NewAuthDecorator("test", "password") - requestFactory.AddDecorator(ad) - - if l := len(requestFactory.GetDecorators()); l != 1 { - t.Fatalf("Expected to have one decorators, got %d", l) - } -} - -func TestRequestFactoryNil(t *testing.T) { - var requestFactory RequestFactory - _, err := requestFactory.NewRequest("GET", "/test", strings.NewReader("test")) - if err != nil { - t.Fatalf("Expected not to get and error, got %s", err) - } -} diff --git a/useragent/README.md b/useragent/README.md new file mode 100644 index 0000000..d9cb367 --- /dev/null +++ b/useragent/README.md @@ -0,0 +1 @@ +This package provides helper functions to pack version information into a single User-Agent header. diff --git a/useragent/useragent.go b/useragent/useragent.go new file mode 100644 index 0000000..9e35d1c --- /dev/null +++ b/useragent/useragent.go @@ -0,0 +1,60 @@ +// Package useragent provides helper functions to pack +// version information into a single User-Agent header. +package useragent + +import ( + "errors" + "strings" +) + +var ( + ErrNilRequest = errors.New("request cannot be nil") +) + +// VersionInfo is used to model UserAgent versions. +type VersionInfo struct { + Name string + Version string +} + +func (vi *VersionInfo) isValid() bool { + const stopChars = " \t\r\n/" + name := vi.Name + vers := vi.Version + if len(name) == 0 || strings.ContainsAny(name, stopChars) { + return false + } + if len(vers) == 0 || strings.ContainsAny(vers, stopChars) { + return false + } + return true +} + +// Convert versions to a string and append the string to the string base. +// +// Each VersionInfo will be converted to a string in the format of +// "product/version", where the "product" is get from the name field, while +// version is get from the version field. Several pieces of verson information +// will be concatinated and separated by space. +// +// Example: +// AppendVersions("base", VersionInfo{"foo", "1.0"}, VersionInfo{"bar", "2.0"}) +// results in "base foo/1.0 bar/2.0". +func AppendVersions(base string, versions ...VersionInfo) string { + if len(versions) == 0 { + return base + } + + verstrs := make([]string, 0, 1+len(versions)) + if len(base) > 0 { + verstrs = append(verstrs, base) + } + + for _, v := range versions { + if !v.isValid() { + continue + } + verstrs = append(verstrs, v.Name+"/"+v.Version) + } + return strings.Join(verstrs, " ") +} diff --git a/useragent/useragent_test.go b/useragent/useragent_test.go new file mode 100644 index 0000000..0ad7243 --- /dev/null +++ b/useragent/useragent_test.go @@ -0,0 +1,31 @@ +package useragent + +import "testing" + +func TestVersionInfo(t *testing.T) { + vi := VersionInfo{"foo", "bar"} + if !vi.isValid() { + t.Fatalf("VersionInfo should be valid") + } + vi = VersionInfo{"", "bar"} + if vi.isValid() { + t.Fatalf("Expected VersionInfo to be invalid") + } + vi = VersionInfo{"foo", ""} + if vi.isValid() { + t.Fatalf("Expected VersionInfo to be invalid") + } +} + +func TestAppendVersions(t *testing.T) { + vis := []VersionInfo{ + {"foo", "1.0"}, + {"bar", "0.1"}, + {"pi", "3.1.4"}, + } + v := AppendVersions("base", vis...) + expect := "base foo/1.0 bar/0.1 pi/3.1.4" + if v != expect { + t.Fatalf("expected %q, got %q", expect, v) + } +} From 8a732f53f7ab6a35c67ff6c571a5615672b959f3 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Fri, 15 May 2015 18:35:04 -0700 Subject: [PATCH 3/3] Add transport package to support CancelRequest Signed-off-by: Tibor Vass --- transport/LICENSE | 27 ++++++++ transport/transport.go | 148 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 175 insertions(+) create mode 100644 transport/LICENSE create mode 100644 transport/transport.go diff --git a/transport/LICENSE b/transport/LICENSE new file mode 100644 index 0000000..d02f24f --- /dev/null +++ b/transport/LICENSE @@ -0,0 +1,27 @@ +Copyright (c) 2009 The oauth2 Authors. All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the following disclaimer +in the documentation and/or other materials provided with the +distribution. + * Neither the name of Google Inc. nor the names of its +contributors may be used to endorse or promote products derived from +this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/transport/transport.go b/transport/transport.go new file mode 100644 index 0000000..510d8b4 --- /dev/null +++ b/transport/transport.go @@ -0,0 +1,148 @@ +package transport + +import ( + "io" + "net/http" + "sync" +) + +type RequestModifier interface { + ModifyRequest(*http.Request) error +} + +type headerModifier http.Header + +// NewHeaderRequestModifier returns a RequestModifier that merges the HTTP headers +// passed as an argument, with the HTTP headers of a request. +// +// If the same key is present in both, the modifying header values for that key, +// are appended to the values for that same key in the request header. +func NewHeaderRequestModifier(header http.Header) RequestModifier { + return headerModifier(header) +} + +func (h headerModifier) ModifyRequest(req *http.Request) error { + for k, s := range http.Header(h) { + req.Header[k] = append(req.Header[k], s...) + } + + return nil +} + +// NewTransport returns an http.RoundTripper that modifies requests according to +// the RequestModifiers passed in the arguments, before sending the requests to +// the base http.RoundTripper (which, if nil, defaults to http.DefaultTransport). +func NewTransport(base http.RoundTripper, modifiers ...RequestModifier) http.RoundTripper { + return &transport{ + Modifiers: modifiers, + Base: base, + } +} + +// transport is an http.RoundTripper that makes HTTP requests after +// copying and modifying the request +type transport struct { + Modifiers []RequestModifier + Base http.RoundTripper + + mu sync.Mutex // guards modReq + modReq map[*http.Request]*http.Request // original -> modified +} + +func (t *transport) RoundTrip(req *http.Request) (*http.Response, error) { + req2 := CloneRequest(req) + for _, modifier := range t.Modifiers { + if err := modifier.ModifyRequest(req2); err != nil { + return nil, err + } + } + + t.setModReq(req, req2) + res, err := t.base().RoundTrip(req2) + if err != nil { + t.setModReq(req, nil) + return nil, err + } + res.Body = &OnEOFReader{ + Rc: res.Body, + Fn: func() { t.setModReq(req, nil) }, + } + return res, nil +} + +// CancelRequest cancels an in-flight request by closing its connection. +func (t *transport) CancelRequest(req *http.Request) { + type canceler interface { + CancelRequest(*http.Request) + } + if cr, ok := t.base().(canceler); ok { + t.mu.Lock() + modReq := t.modReq[req] + delete(t.modReq, req) + t.mu.Unlock() + cr.CancelRequest(modReq) + } +} + +func (t *transport) base() http.RoundTripper { + if t.Base != nil { + return t.Base + } + return http.DefaultTransport +} + +func (t *transport) setModReq(orig, mod *http.Request) { + t.mu.Lock() + defer t.mu.Unlock() + if t.modReq == nil { + t.modReq = make(map[*http.Request]*http.Request) + } + if mod == nil { + delete(t.modReq, orig) + } else { + t.modReq[orig] = mod + } +} + +// CloneRequest returns a clone of the provided *http.Request. +// The clone is a shallow copy of the struct and its Header map. +func CloneRequest(r *http.Request) *http.Request { + // shallow copy of the struct + r2 := new(http.Request) + *r2 = *r + // deep copy of the Header + r2.Header = make(http.Header, len(r.Header)) + for k, s := range r.Header { + r2.Header[k] = append([]string(nil), s...) + } + + return r2 +} + +// OnEOFReader ensures a callback function is called +// on Close() and when the underlying Reader returns an io.EOF error +type OnEOFReader struct { + Rc io.ReadCloser + Fn func() +} + +func (r *OnEOFReader) Read(p []byte) (n int, err error) { + n, err = r.Rc.Read(p) + if err == io.EOF { + r.runFunc() + } + return +} + +func (r *OnEOFReader) Close() error { + err := r.Rc.Close() + r.runFunc() + return err +} + +func (r *OnEOFReader) runFunc() { + if fn := r.Fn; fn != nil { + fn() + r.Fn = nil + } +}