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

Jump to bottom

Added catch for missing POST body when posted to profileparameters #7194

Merged
merged 11 commits into from
Nov 22, 2022
Merged

Added catch for missing POST body when posted to profileparameters #7194

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
78afe26
Added catch for missing POST body when posted to profileparameters
kdamichie Nov 14, 2022
87e4686
Added test for missing POST body to profileparameters into v3 and v5,…
kdamichie Nov 14, 2022
754a27a
Delete traffic-ops-test.conf
kdamichie Nov 15, 2022
cd2b3fa
Accidentally deleted. Meant to revert. Adding back in.
kdamichie Nov 15, 2022
a1e28f6
Merge changes
kdamichie Nov 15, 2022
903e613
Update traffic-ops-test.conf
kdamichie Nov 15, 2022
cd37d00
Merge branch 'master' into fix_profileparameters_internal_server_error
kdamichie Nov 16, 2022
ffbca69
Merge branch 'fix_profileparameters_internal_server_error' of github.…
kdamichie Nov 16, 2022
5fc7ec2
Created one off test to check POST for an empty JSON body
kdamichie Nov 18, 2022
1577f83
Merge branch 'master' into fix_profileparameters_internal_server_error
kdamichie Nov 18, 2022
8d05b3b
Corrected correct URL Path for versions 3.0 and 4.0
kdamichie Nov 21, 2022
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- [#7037](https://github.com/apache/trafficcontrol/pull/7037) *Traffic Router* Uses Traffic Ops API 4.0 by default

### Fixed
- [#4428](https://github.com/apache/trafficcontrol/issues/4428) *Traffic Ops* Fixed Internal Server Error with POST to `profileparameters` when POST body is empty
- [#7179](https://github.com/apache/trafficcontrol/issues/7179) *Traffic Portal* Fixed search filter for Delivery Service Table
- [#7174](https://github.com/apache/trafficcontrol/issues/7174) *Traffic Portal* Fixed topologies sort (table and Delivery Service's form)
- [#5970](https://github.com/apache/trafficcontrol/issues/5970) *Traffic Portal* Fixed numeric sort in Delivery Service's form for DSCP
Expand Down
5 changes: 5 additions & 0 deletions traffic_ops/testing/api/v3/profile_parameters_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ func TestProfileParameters(t *testing.T) {
}},
Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
},
"BAD REQUEST when EMPTY BODY PROVIDED": {
ClientSession: TOSession,
RequestBody: []tc.ProfileParameter{{}},
Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
},
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't actually test the behavior being fixed in this PR. This a list of length 1 (not 0!) whose only element has the default/"zero" values for all of its properties. The result when Go marshals that is, accordingly, an array with one object element.

"BAD REQUEST when MISSING PROFILEID field": {
ClientSession: TOSession,
RequestBody: []tc.ProfileParameter{{
Expand Down
5 changes: 5 additions & 0 deletions traffic_ops/testing/api/v4/profile_parameters_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ func TestProfileParameters(t *testing.T) {
},
Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
},
"BAD REQUEST when EMPTY BODY PROVIDED": {
ClientSession: TOSession,
RequestBody: map[string]interface{}{},
Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
},
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't actually test the behavior being fixed in this PR. Marshalling an empty map appropriately produces an empty object - not an empty request body!

"BAD REQUEST when MISSING PROFILEID field": {
ClientSession: TOSession,
RequestBody: map[string]interface{}{
Expand Down
5 changes: 5 additions & 0 deletions traffic_ops/testing/api/v5/profile_parameters_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ func TestProfileParameters(t *testing.T) {
},
Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
},
"BAD REQUEST when EMPTY BODY PROVIDED": {
ClientSession: TOSession,
RequestBody: map[string]interface{}{},
Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

"BAD REQUEST when MISSING PROFILEID field": {
ClientSession: TOSession,
RequestBody: map[string]interface{}{
Expand Down
5 changes: 5 additions & 0 deletions traffic_ops/traffic_ops_golang/api/shared_handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,11 @@ func CreateHandler(creator Creator) http.HandlerFunc {
return
}

if len(data) == 0 {
HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("no request body supplied"), nil)
return
}

objSlice, err := parseMultipleCreates(data, objectType, inf)
if err != nil {
HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err)
Expand Down