Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore: Update max bytes reader middleware error message #1072

Merged
merged 3 commits into from
Sep 8, 2024
Merged

Chore: Update max bytes reader middleware error message #1072

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
07f56fb
update max bytes reader middleware error message
itsmylife Sep 6, 2024
6441e0b
attempt to improve the naming
itsmylife Sep 6, 2024
63f106f
rename
itsmylife Sep 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions backend/httpclient/max_bytes_reader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,13 @@ var ErrResponseBodyTooLarge = errors.New("http: response body too large")
// MaxBytesReader prevents clients from accidentally or maliciously
// sending a large request and wasting server resources.
func MaxBytesReader(r io.ReadCloser, n int64) io.ReadCloser {
return &maxBytesReader{r: r, n: n}
return &maxBytesReader{r: r, n: n, a: n}
}

type maxBytesReader struct {
r io.ReadCloser // underlying reader
n int64 // max bytes remaining
a int64 // the actual limit
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit. Naming could be improved for these fields :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marefr An attempt has been made here 6441e0b but I am not sure if those are good enough.

err error // sticky error
}

Expand Down Expand Up @@ -57,7 +58,7 @@ func (l *maxBytesReader) Read(p []byte) (n int, err error) {
n = int(l.n)
l.n = 0

l.err = fmt.Errorf("error: %w, response limit is set to: %d", ErrResponseBodyTooLarge, n)
l.err = fmt.Errorf("error: %w, response limit is set to: %d", ErrResponseBodyTooLarge, l.a)
return n, l.err
}

Expand Down
20 changes: 10 additions & 10 deletions backend/httpclient/max_bytes_reader_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ import (

func TestMaxBytesReader(t *testing.T) {
tcs := []struct {
limit int64
bodyLength int
body string
err error
limit int64
expectedBodyLength int
expectedBody string
Comment on lines +16 to +17
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Helps me to create a mental model when I read the tests

err error
}{
{limit: 1, bodyLength: 1, body: "d", err: errors.New("error: http: response body too large, response limit is set to: 1")},
{limit: 1000000, bodyLength: 5, body: "dummy", err: nil},
{limit: 0, bodyLength: 0, body: "", err: errors.New("error: http: response body too large, response limit is set to: 0")},
{limit: 1, expectedBodyLength: 1, expectedBody: "d", err: errors.New("error: http: response body too large, response limit is set to: 1")},
{limit: 1000000, expectedBodyLength: 5, expectedBody: "dummy", err: nil},
{limit: 0, expectedBodyLength: 0, expectedBody: "", err: errors.New("error: http: response body too large, response limit is set to: 0")},
}
for _, tc := range tcs {
t.Run(fmt.Sprintf("Test MaxBytesReader with limit: %d", tc.limit), func(t *testing.T) {
Expand All @@ -28,13 +28,13 @@ func TestMaxBytesReader(t *testing.T) {

bodyBytes, err := io.ReadAll(readCloser)
if err != nil {
require.EqualError(t, tc.err, err.Error())
require.EqualError(t, err, tc.err.Error())
} else {
require.NoError(t, tc.err)
}

require.Len(t, bodyBytes, tc.bodyLength)
require.Equal(t, string(bodyBytes), tc.body)
require.Len(t, bodyBytes, tc.expectedBodyLength)
require.Equal(t, tc.expectedBody, string(bodyBytes))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I replaced the expected and the actual values so the failed test message can help me with what's expected and what's actual value.

})
}
}
18 changes: 9 additions & 9 deletions backend/httpclient/response_limit_middleware_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ import (

func TestResponseLimitMiddleware(t *testing.T) {
tcs := []struct {
limit int64
bodyLength int
body string
err error
limit int64
expectedBodyLength int
expectedBody string
err error
}{
{limit: 1, bodyLength: 1, body: "d", err: errors.New("error: http: response body too large, response limit is set to: 1")},
{limit: 1000000, bodyLength: 5, body: "dummy", err: nil},
{limit: 0, bodyLength: 5, body: "dummy", err: nil},
{limit: 1, expectedBodyLength: 1, expectedBody: "d", err: errors.New("error: http: response body too large, response limit is set to: 1")},
{limit: 1000000, expectedBodyLength: 5, expectedBody: "dummy", err: nil},
{limit: 0, expectedBodyLength: 5, expectedBody: "dummy", err: nil},
}
for _, tc := range tcs {
t.Run(fmt.Sprintf("Test ResponseLimitMiddleware with limit: %d", tc.limit), func(t *testing.T) {
Expand Down Expand Up @@ -51,8 +51,8 @@ func TestResponseLimitMiddleware(t *testing.T) {
}
require.NoError(t, res.Body.Close())

require.Len(t, bodyBytes, tc.bodyLength)
require.Equal(t, string(bodyBytes), tc.body)
require.Len(t, bodyBytes, tc.expectedBodyLength)
require.Equal(t, string(bodyBytes), tc.expectedBody)
})
}
}