From 8b32cd6f52e32195f15b55c5217c12c29d6004b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maxime=20Soul=C3=A9?= Date: Thu, 20 Jul 2023 11:19:00 +0200 Subject: [PATCH] fix: BodyContainsBytes & BodyContainsString used with And/Or MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both "rearm" the body before (re-)reading it, so they can be used several times in a same matcher (combined with Or & And). At the same time, simplify the body copy and make the "rearming" more performant. Closes #145. Signed-off-by: Maxime Soulé --- export_test.go | 4 ---- match.go | 43 ++++++++++++++++++++++-------------- match_test.go | 55 ++++++++++++++++------------------------------- transport_test.go | 21 ++++++++++++++++-- 4 files changed, 65 insertions(+), 58 deletions(-) diff --git a/export_test.go b/export_test.go index e9c2aff..b795529 100644 --- a/export_test.go +++ b/export_test.go @@ -38,10 +38,6 @@ func (b *bodyCopyOnRead) Body() io.ReadCloser { return b.body } -func (b *bodyCopyOnRead) Buf() []byte { - return b.buf -} - func (b *bodyCopyOnRead) Rearm() { b.rearm() } diff --git a/match.go b/match.go index baf46ee..ba6ada4 100644 --- a/match.go +++ b/match.go @@ -106,6 +106,7 @@ type MatcherFunc func(req *http.Request) bool func matcherFuncOr(mfs []MatcherFunc) MatcherFunc { return func(req *http.Request) bool { for _, mf := range mfs { + rearmBody(req) if mf(req) { return true } @@ -120,6 +121,7 @@ func matcherFuncAnd(mfs []MatcherFunc) MatcherFunc { } return func(req *http.Request) bool { for _, mf := range mfs { + rearmBody(req) if !mf(req) { return false } @@ -224,6 +226,7 @@ func NewMatcher(name string, fn MatcherFunc) Matcher { func BodyContainsBytes(subslice []byte) Matcher { return NewMatcher("", func(req *http.Request) bool { + rearmBody(req) b, err := ioutil.ReadAll(req.Body) return err == nil && bytes.Contains(b, subslice) }) @@ -243,6 +246,7 @@ func BodyContainsBytes(subslice []byte) Matcher { func BodyContainsString(substr string) Matcher { return NewMatcher("", func(req *http.Request) bool { + rearmBody(req) b, err := ioutil.ReadAll(req.Body) return err == nil && bytes.Contains(b, []byte(substr)) }) @@ -465,27 +469,40 @@ func (m matchRouteKey) String() string { return m.RouteKey.String() + " <" + m.name + ">" } -// bodyCopyOnRead copies body content to buf on first Read(), except +func rearmBody(req *http.Request) { + if req != nil { + if body, ok := req.Body.(interface{ rearm() }); ok { + body.rearm() + } + } +} + +type buffer struct { + *bytes.Reader +} + +func (b buffer) Close() error { + return nil +} + +// bodyCopyOnRead mutates body into a buffer on first Read(), except // if body is nil or http.NoBody. In this case, EOF is returned for -// each Read() and buf stays to nil. +// each Read() and body stays untouched. type bodyCopyOnRead struct { body io.ReadCloser - buf []byte } func (b *bodyCopyOnRead) rearm() { - if b.buf != nil { - b.body = ioutil.NopCloser(bytes.NewReader(b.buf)) + if buf, ok := b.body.(buffer); ok { + buf.Seek(0, io.SeekStart) //nolint:errcheck } // else b.body contains the original body, so don't touch } func (b *bodyCopyOnRead) copy() { - if b.buf == nil && b.body != nil && b.body != http.NoBody { - var body bytes.Buffer - io.Copy(&body, b.body) //nolint: errcheck + if _, ok := b.body.(buffer); !ok && b.body != nil && b.body != http.NoBody { + buf, _ := ioutil.ReadAll(b.body) b.body.Close() - b.buf = body.Bytes() - b.body = ioutil.NopCloser(bytes.NewReader(b.buf)) + b.body = buffer{bytes.NewReader(buf)} } } @@ -500,9 +517,3 @@ func (b *bodyCopyOnRead) Read(p []byte) (n int, err error) { func (b *bodyCopyOnRead) Close() error { return nil } - -// Len returns the buffer total length, whatever the Read position in body is. -func (b *bodyCopyOnRead) Len() int { - b.copy() - return len(b.buf) -} diff --git a/match_test.go b/match_test.go index 2984d79..737683b 100644 --- a/match_test.go +++ b/match_test.go @@ -84,11 +84,20 @@ func TestNewMatcher(t *testing.T) { return req } + reqCopyBody := func(t testing.TB, body string, header ...string) *http.Request { + req := req(t, body, header...) + req.Body = httpmock.NewBodyCopyOnRead(req.Body) + return req + } + t.Run("BodyContainsBytes", func(t *testing.T) { m := httpmock.BodyContainsBytes([]byte("ip")) td.Cmp(t, m.Name(), autogenName) td.CmpTrue(t, m.Check(req(t, "pipo"))) td.CmpFalse(t, m.Check(req(t, "bingo"))) + + td.CmpTrue(t, m.Check(reqCopyBody(t, "pipo"))) + td.CmpFalse(t, m.Check(reqCopyBody(t, "bingo"))) }) t.Run("BodyContainsString", func(t *testing.T) { @@ -96,6 +105,9 @@ func TestNewMatcher(t *testing.T) { td.Cmp(t, m.Name(), autogenName) td.CmpTrue(t, m.Check(req(t, "pipo"))) td.CmpFalse(t, m.Check(req(t, "bingo"))) + + td.CmpTrue(t, m.Check(reqCopyBody(t, "pipo"))) + td.CmpFalse(t, m.Check(reqCopyBody(t, "bingo"))) }) t.Run("HeaderExists", func(t *testing.T) { @@ -394,7 +406,7 @@ func TestBodyCopyOnRead(t *testing.T) { bc := httpmock.NewBodyCopyOnRead(body) bc.Rearm() - td.CmpNil(t, bc.Buf()) + td.Cmp(t, body, bc.Body(), "rearm didn't touch anything") var buf [4]byte n, err := bc.Read(buf[:]) @@ -402,7 +414,9 @@ func TestBodyCopyOnRead(t *testing.T) { td.Cmp(t, n, 4) td.CmpString(t, buf[:], "BODY") - td.CmpString(t, bc.Buf(), "BODY", "Original body has been copied internally") + td.Cmp(t, body, td.Not(bc.Body()), "Original body has been copied internally") + + td.CmpNoError(t, bc.Body().Close()) // for coverage... :) n, err = bc.Read(buf[:]) td.Cmp(t, err, io.EOF) @@ -435,55 +449,24 @@ func TestBodyCopyOnRead(t *testing.T) { bc := httpmock.NewBodyCopyOnRead(tc.body) bc.Rearm() - td.CmpNil(t, bc.Buf()) + td.Cmp(t, tc.body, bc.Body(), "rearm didn't touch anything") var buf [4]byte n, err := bc.Read(buf[:]) td.Cmp(t, err, io.EOF) td.Cmp(t, n, 0) - td.CmpNil(t, bc.Buf()) - td.Cmp(t, bc.Body(), tc.body) + td.Cmp(t, bc.Body(), tc.body, "body is not altered") bc.Rearm() n, err = bc.Read(buf[:]) td.Cmp(t, err, io.EOF) td.Cmp(t, n, 0) - td.CmpNil(t, bc.Buf()) - td.Cmp(t, bc.Body(), tc.body) + td.Cmp(t, bc.Body(), tc.body, "body is not altered") td.CmpNoError(t, bc.Close()) }) } - - t.Run("len", func(t *testing.T) { - testCases := []struct { - name string - bc interface{ Len() int } - expected int - }{ - { - name: "nil", - bc: httpmock.NewBodyCopyOnRead(nil), - expected: 0, - }, - { - name: "no body", - bc: httpmock.NewBodyCopyOnRead(http.NoBody), - expected: 0, - }, - { - name: "filled", - bc: httpmock.NewBodyCopyOnRead(ioutil.NopCloser(bytes.NewReader([]byte(`BODY`)))), - expected: 4, - }, - } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - td.Cmp(t, tc.bc.Len(), tc.expected) - }) - } - }) } func TestExtractPackage(t *testing.T) { diff --git a/transport_test.go b/transport_test.go index e2aa6cd..a2d7a6e 100644 --- a/transport_test.go +++ b/transport_test.go @@ -123,6 +123,12 @@ func TestRegisterMatcherResponder(t *testing.T) { }), NewStringResponder(200, "body-FOO")) + RegisterMatcherResponder("POST", "/foo", + BodyContainsString("xxx"). + Or(BodyContainsString("yyy")). + WithName("03-body-xxx|yyy"), + NewStringResponder(200, "body-xxx|yyy")) + RegisterResponder("POST", "/foo", NewStringResponder(200, "default")) RegisterNoResponder(NewNotFoundResponder(nil)) @@ -156,6 +162,16 @@ func TestRegisterMatcherResponder(t *testing.T) { body: "FOO", expectedBody: "body-FOO", }, + { + name: "body xxx", + body: "...xxx...", + expectedBody: "body-xxx|yyy", + }, + { + name: "body yyy", + body: "...yyy...", + expectedBody: "body-xxx|yyy", + }, { name: "default", body: "ANYTHING", @@ -194,12 +210,13 @@ func TestRegisterMatcherResponder(t *testing.T) { "text/plain", strings.NewReader("ANYTHING"), ) - assert.HasSuffix(err, `Responder not found for POST http://test.com/foo despite 3 matchers: ["00-header-foo=bar" "01-body-BAR" "02-body-FOO"]`) + assert.HasSuffix(err, `Responder not found for POST http://test.com/foo despite 4 matchers: ["00-header-foo=bar" "01-body-BAR" "02-body-FOO" "03-body-xxx|yyy"]`) }) - // Remove 2 matcher responders + // Remove 3 matcher responders RegisterMatcherResponder("POST", "/foo", NewMatcher("01-body-BAR", nil), nil) RegisterMatcherResponder("POST", "/foo", NewMatcher("02-body-FOO", nil), nil) + RegisterMatcherResponder("POST", "/foo", NewMatcher("03-body-xxx|yyy", nil), nil) assert.Run("not found despite 1", func(assert *td.T) { _, err := http.Post(