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.

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

Allow empty content type on DELETE #73

Merged
merged 1 commit into from
May 10, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions bindata.go

Large diffs are not rendered by default.

16 changes: 14 additions & 2 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,26 @@ func initTestSpec() {
Content: map[string]spec.MediaType{
"application/json": {
Schema: &spec.Schema{
Ref: "#/components/schemas/customer",
Ref: "#/components/schemas/charge",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This tweaks is an unrelated fix.

},
},
},
},
},
}
chargeDeleteMethod = &spec.Operation{
Responses: map[spec.StatusCode]spec.Response{
"200": {
Content: map[string]spec.MediaType{
"application/json": {
Schema: &spec.Schema{
Ref: "#/components/schemas/charge",
},
},
},
},
},
}
chargeDeleteMethod = &spec.Operation{}
chargeGetMethod = &spec.Operation{}

// Here so we can test the relatively rare "action" operations (e.g.,
Expand Down
2 changes: 1 addition & 1 deletion openapi
121 changes: 74 additions & 47 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,53 +183,13 @@ func (s *StubServer) HandleRequest(w http.ResponseWriter, r *http.Request) {
}
}

// Currently we only validate parameters in the request body, but we should
// really validate query and URL parameters as well now that we've
// transitioned to OpenAPI 3.0
mediaType, bodySchema := getRequestBodySchema(route.operation)
if mediaType != nil {
contentType := r.Header.Get("Content-Type")

if contentType == "" {
message := fmt.Sprintf(contentTypeEmpty, *mediaType)
fmt.Printf(message + "\n")
stripeError := createStripeError(typeInvalidRequestError, message)
writeResponse(w, r, start, http.StatusBadRequest, stripeError)
return
}

// Truncate content type parameters. For example, given:
//
// application/json; charset=utf-8
//
// We want to chop off the `; charset=utf-8` at the end.
contentType = strings.Split(contentType, ";")[0]

if contentType != *mediaType {
message := fmt.Sprintf(contentTypeMismatched, *mediaType, contentType)
fmt.Printf(message + "\n")
stripeError := createStripeError(typeInvalidRequestError, message)
writeResponse(w, r, start, http.StatusBadRequest, stripeError)
return
}

err := coercer.CoerceParams(bodySchema, requestData)
if err != nil {
message := fmt.Sprintf("Request coercion error: %v", err)
fmt.Printf(message + "\n")
stripeError := createStripeError(typeInvalidRequestError, message)
writeResponse(w, r, start, http.StatusBadRequest, stripeError)
return
}

err = route.requestBodyValidator.Validate(requestData)
if err != nil {
message := fmt.Sprintf("Request validation error: %v", err)
fmt.Printf(message + "\n")
stripeError := createStripeError(typeInvalidRequestError, message)
writeResponse(w, r, start, http.StatusBadRequest, stripeError)
return
}
// Note that requestData is actually manipulated in place, but we show it
// returned here to make it clear that this function will be manipulating
// it.
requestData, stripeError := validateAndCoerceRequest(r, route, requestData)
if stripeError != nil {
writeResponse(w, r, start, http.StatusBadRequest, stripeError)
return
}

expansions, rawExpansions := extractExpansions(requestData)
Expand Down Expand Up @@ -586,6 +546,73 @@ func parseExpansionLevel(raw []string) *ExpansionLevel {
return level
}

// validateAndCoerceRequest validates an incoming request against an OpenAPI
// schema and does parameter coercion.
//
// Firstly, `Content-Type` is checked against the schema's media type, then
// string-encoded parameters are coerced to expected types (where possible).
// Finally, we validate the incoming payload against the schema.
func validateAndCoerceRequest(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of this code is the same, but for cleanliness I've pulled it into its own function.

r *http.Request,
route *stubServerRoute,
requestData map[string]interface{}) (map[string]interface{}, *ResponseError) {

// Currently we only validate parameters in the request body, but we should
// really validate query and URL parameters as well now that we've
// transitioned to OpenAPI 3.0.
mediaType, bodySchema := getRequestBodySchema(route.operation)

// There are no parameters on this route to validate.
if mediaType == nil {
return requestData, nil
}

contentType := r.Header.Get("Content-Type")

if contentType == "" {
// A special case: if a `DELETE` operation comes in with no request
// payload, we allow it. Most `DELETE` operations take no parameters,
// but a few of them take some optional ones.
if r.Method == http.MethodDelete {
return requestData, nil
}

message := fmt.Sprintf(contentTypeEmpty, *mediaType)
fmt.Printf(message + "\n")
return nil, createStripeError(typeInvalidRequestError, message)
}

// Truncate content type parameters. For example, given:
//
// application/json; charset=utf-8
//
// We want to chop off the `; charset=utf-8` at the end.
contentType = strings.Split(contentType, ";")[0]

if contentType != *mediaType {
message := fmt.Sprintf(contentTypeMismatched, *mediaType, contentType)
fmt.Printf(message + "\n")
return nil, createStripeError(typeInvalidRequestError, message)
}

err := coercer.CoerceParams(bodySchema, requestData)
if err != nil {
message := fmt.Sprintf("Request coercion error: %v", err)
fmt.Printf(message + "\n")
return nil, createStripeError(typeInvalidRequestError, message)
}

err = route.requestBodyValidator.Validate(requestData)
if err != nil {
message := fmt.Sprintf("Request validation error: %v", err)
fmt.Printf(message + "\n")
return nil, createStripeError(typeInvalidRequestError, message)
}

// All checks were successful.
return requestData, nil
}

func validateAuth(auth string) bool {
if auth == "" {
return false
Expand Down
12 changes: 10 additions & 2 deletions server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,14 @@ func TestStubServer_ErrorsOnEmptyContentType(t *testing.T) {
errorInfo["message"])
}

func TestStubServer_AllowsEmptyContentTypeOnDelete(t *testing.T) {
headers := getDefaultHeaders()
headers["Content-Type"] = ""

resp, _ := sendRequest(t, "DELETE", "/v1/charges/ch_123", "", headers)
assert.Equal(t, http.StatusOK, resp.StatusCode)
}

func TestStubServer_ErrorsOnMismatchedContentType(t *testing.T) {
headers := getDefaultHeaders()
headers["Content-Type"] = "application/json"
Expand Down Expand Up @@ -178,7 +186,7 @@ func TestStubServer_RoutesRequest(t *testing.T) {
&http.Request{Method: "POST",
URL: &url.URL{Path: "/v1/invoices/in_123/pay"}})
assert.NotNil(t, route)
assert.Equal(t, chargeDeleteMethod, route.operation)
assert.Equal(t, invoicePayMethod, route.operation)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

My tweaks to the test fixtures revealed that these were previously wrong, so they've also been fixed.

assert.Equal(t, "in_123", *(*pathParams).PrimaryID)
assert.Equal(t, []*PathParamsSecondaryID(nil), (*pathParams).SecondaryIDs)
}
Expand All @@ -202,7 +210,7 @@ func TestStubServer_RoutesRequest(t *testing.T) {
&http.Request{Method: "GET",
URL: &url.URL{Path: "/v1/application_fees/fee_123/refunds/fr_123"}})
assert.NotNil(t, route)
assert.Equal(t, chargeDeleteMethod, route.operation)
assert.Equal(t, applicationFeeRefundGetMethod, route.operation)
assert.Equal(t, "fr_123", *(*pathParams).PrimaryID)
assert.Equal(t, 1, len((*pathParams).SecondaryIDs))
assert.Equal(t, "fee_123", (*pathParams).SecondaryIDs[0].ID)
Expand Down