Allow empty content type on DELETE

We recently added checks in `stripe-mock` to make sure that the right
`Content-Type` was being sent. Unfortunately, the logic was not quite
right for deletes, where you occasionally see a `Content-Type` in rare
cases, but which usually have empty request bodies, and therefore an
empty `Content-Type` is allowed.

This patch brings in two fixes:

1. The OpenAPI spec has been tweak so as to not incorrectly include
   `expand` parameters on `DELETE` operations. This means that most
   `DELETE` operations have no defined request schema at all.
2. Special cases incoming `DELETE` operations so that they can be
   checked against a schema if they provide parameters, but are also
   allowed to specify an empty `Content-Type` as well.

Between the two improvements, all edges should be covered.

The impetus here is to resolve [1] where a `stripe-mock` upgrade is
incompatible with the current code. I've verified that with this patch
included, `stripe-python`'s test suite runs successfully.

[1] stripe/stripe-python#426

main_test.go

@@ -83,14 +83,26 @@ func initTestSpec() {
 				Content: map[string]spec.MediaType{
 					"application/json": {
 						Schema: &spec.Schema{
-							Ref: "#/components/schemas/customer",
+							Ref: "#/components/schemas/charge",
+						},
+					},
+				},
+			},
+		},
+	}
+	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.,

server.go

@@ -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)
@@ -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(
+	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

server_test.go

@@ -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"
@@ -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)
 		assert.Equal(t, "in_123", *(*pathParams).PrimaryID)
 		assert.Equal(t, []*PathParamsSecondaryID(nil), (*pathParams).SecondaryIDs)
 	}
@@ -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)