Elgato_dark/fleet
1
0
Fork 0
mirror of https://github.com/fleetdm/fleet synced 2026-05-23 17:08:53 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 83

For requests with invalid list options, return 400 instead of 500 (#11632)

Browse source 
## Addresses #11272 

- For requests with invalid list options (`page`, `per_page`,
`order_key`, `order_direction`), return `400` instead of `500`
<img width="957" alt="Screenshot 2023-05-10 at 2 28 56 PM"
src="https://github.com/fleetdm/fleet/assets/61553566/d4400a92-b158-4a41-9d00-9ba5170d48f6">

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

- [x] Manual QA for all new/changed functionality

---------

Co-authored-by: Jacob Shandling <jacob@fleetdm.com>
This commit is contained in:
Jacob Shandling 2023-05-17 13:41:30 -07:00 committed by GitHub
parent 3f9eccc7f8
commit 49b04ba4a5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
server/service/transport.go
View file

@ -130,10 +130,10 @@ func listOptionsFromRequest(r *http.Request) (fleet.ListOptions, error) {
if pageString != "" {
page, err = strconv.Atoi(pageString)
if err != nil {
return fleet.ListOptions{}, ctxerr.New(r.Context(), "non-int page value")
return fleet.ListOptions{}, ctxerr.Wrap(r.Context(), badRequest("non-int page value"))
}
if page < 0 {
return fleet.ListOptions{}, ctxerr.New(r.Context(), "negative page value")
return fleet.ListOptions{}, ctxerr.Wrap(r.Context(), badRequest("negative page value"))
}
}
@ -143,10 +143,10 @@ func listOptionsFromRequest(r *http.Request) (fleet.ListOptions, error) {
if perPageString != "" {
perPage, err = strconv.Atoi(perPageString)
if err != nil {
return fleet.ListOptions{}, ctxerr.New(r.Context(), "non-int per_page value")
return fleet.ListOptions{}, ctxerr.Wrap(r.Context(), badRequest("non-int per_page value"))
}
if perPage <= 0 {
return fleet.ListOptions{}, ctxerr.New(r.Context(), "invalid per_page value")
return fleet.ListOptions{}, ctxerr.Wrap(r.Context(), badRequest("invalid per_page value"))
}
}
@ -158,8 +158,7 @@ func listOptionsFromRequest(r *http.Request) (fleet.ListOptions, error) {
}
if orderKey == "" && orderDirectionString != "" {
return fleet.ListOptions{},
ctxerr.New(r.Context(), "order_key must be specified with order_direction")
return fleet.ListOptions{}, ctxerr.Wrap(r.Context(), badRequest("order_key must be specified with order_direction"))
}
var orderDirection fleet.OrderDirection
@ -172,7 +171,7 @@ func listOptionsFromRequest(r *http.Request) (fleet.ListOptions, error) {
orderDirection = fleet.OrderAscending
default:
return fleet.ListOptions{},
ctxerr.New(r.Context(), "unknown order_direction: "+orderDirectionString)
ctxerr.Wrap(r.Context(), badRequest("unknown order_direction: "+orderDirectionString))
}

35
server/service/transport_test.go
View file

@ -7,6 +7,7 @@ import (
"github.com/fleetdm/fleet/v4/server/fleet"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestListOptionsFromRequest(t *testing.T) {
@ -15,8 +16,8 @@ func TestListOptionsFromRequest(t *testing.T) {
url string
// expected list options
listOptions fleet.ListOptions
// should cause an error
shouldErr bool
// should cause a BadRequest error
shouldErr400 bool
}{
// both params provided
{
@ -67,30 +68,30 @@ func TestListOptionsFromRequest(t *testing.T) {
},
},
// various error cases
// various 400 error cases
{
url: "/foo?page=foo&per_page=10",
shouldErr: true,
url: "/foo?page=foo&per_page=10",
shouldErr400: true,
},
{
url: "/foo?page=1&per_page=foo",
shouldErr: true,
url: "/foo?page=1&per_page=foo",
shouldErr400: true,
},
{
url: "/foo?page=-1",
shouldErr: true,
url: "/foo?page=-1",
shouldErr400: true,
},
{
url: "/foo?page=-1&per_page=-10",
shouldErr: true,
url: "/foo?page=-1&per_page=-10",
shouldErr400: true,
},
{
url: "/foo?page=1&order_direction=desc",
shouldErr: true,
url: "/foo?page=1&order_direction=desc",
shouldErr400: true,
},
{
url: "/foo?&order_direction=foo&order_key=",
shouldErr: true,
url: "/foo?&order_direction=foo&order_key=",
shouldErr400: true,
},
}
@ -100,8 +101,10 @@ func TestListOptionsFromRequest(t *testing.T) {
req := &http.Request{URL: url}
opt, err := listOptionsFromRequest(req)
if tt.shouldErr {
if tt.shouldErr400 {
assert.NotNil(t, err)
var be *fleet.BadRequestError
require.ErrorAs(t, err, &be)
return
}