validation adds default values to request body

[orensolo]

Hi,

I am using the latest version of kin-openapi (v0.106.0).
In this latest version, kin-openapi adds default values to the request body even if they are not present.
For example, if the schema contains :

prop1:
          default: false
          type: boolean

Then "prop1=false" will be added to the request body if not present.

This is wrong:
According to openapi specification https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#schema-object:

default - The default value represents what would be assumed by the consumer of the input as the value of the schema if one is not provided.

Meaning that the consumer should make the assumptions, not the producer.
Adding these default values just create network overhead by making the requests larger without any need.

I am getting the following error because of that :

http2: request body larger than specified content length

The code responsible for that is (in ValidateRequestBody function in validate_request.go):

	if defaultsSet {
		var err error
		if data, err = encodeBody(value, mediaType); err != nil {
			return &RequestError{
				Input:       input,
				RequestBody: requestBody,
				Reason:      "rewriting failed",
				Err:         err,
			}
		}
		// Put the data back into the input
		req.Body = ioutil.NopCloser(bytes.NewReader(data))
	}

I suggest either to remove this code, or make it optional (and the default behavior should be that no default values are added to the request body).

Thanks,
Oren

[orensolo]

@fenollp can you take a look? what do you think?

[orensolo]

Hi @fenollp (or anyone else maintaining this project).

I suggest a fix to the issue above.

In this fix, I keep the current behavior of adding default values to the request after the validation.
But also makes it configurable when that behavior is not desired.

Please use the fix (if all ok) and also add the validation test (just below the fix).

In options.go I added a new option (SkipDefaultValueSet) :

type Options struct {
	// Set ExcludeRequestBody so ValidateRequest skips request body validation
	ExcludeRequestBody bool

	// Set ExcludeResponseBody so ValidateResponse skips response body validation
	ExcludeResponseBody bool

	// Set IncludeResponseStatus so ValidateResponse fails on response
	// status not defined in OpenAPI spec
	IncludeResponseStatus bool

	MultiError bool

	// See NoopAuthenticationFunc
	AuthenticationFunc AuthenticationFunc
	//SkipDefaultValueSet indicates whether to skip setting default values
	// in the original request body
	SkipDefaultValueSet bool // <=== here is the new option
}

In ValidateRequestBody function in validate_request.go, please add the following code:

	if !options.SkipDefaultValueSet {
		opts = append(opts, openapi3.DefaultsSet(func() { defaultsSet = true }))
	}

instead of

opts = append(opts, openapi3.DefaultsSet(func() { defaultsSet = true }))

Here is a test to validate the fix:

package main

import (
	"context"
	"encoding/json"
	"fmt"
	"io/ioutil"
	"net/http"
	"strings"

	"github.com/getkin/kin-openapi/openapi3"
	"github.com/getkin/kin-openapi/openapi3filter"
	"github.com/getkin/kin-openapi/routers/gorillamux"
)

func main() {
	ctx := context.Background()
	loader := &openapi3.Loader{Context: ctx, IsExternalRefsAllowed: true}
	schema :=
		`
openapi: 3.0.0
info:
 version: 1.0.0
 title: Sample API
paths:
 /items:
  put:
   requestBody:
    content:
      application/json:
        schema:
          properties:
            testWithdefault:
              default: false
              type: boolean
            testNoDefault:
              type: boolean	 
          type: object
         
   responses:
    '200':
     description: Successful response`

	doc, err := loader.LoadFromData([]byte(schema))

	if err != nil {
		fmt.Println("failed to load schema. " + err.Error())
		return
	}
	// Validate document

	if err = doc.Validate(ctx); err != nil {
		fmt.Println("failed to validate schema. " + err.Error())
		return
	}

	router, _ := gorillamux.NewRouter(doc)
	body := "{\"testNoDefault\": true}"
	httpReq, _ := http.NewRequest(http.MethodPut, "/items", strings.NewReader(body))
	httpReq.Header.Set("Content-Type", "application/json")
	// Find route
	route, pathParams, _ := router.FindRoute(httpReq)

	// Validate request
	requestValidationInput := &openapi3filter.RequestValidationInput{
		Request:    httpReq,
		PathParams: pathParams,
		Route:      route,
		Options: &openapi3filter.Options{
			SkipDefaultValueSet: true, // <== false by default, true means doesn't add default values to request body
		},
	}
	err = openapi3filter.ValidateRequest(ctx, requestValidationInput)
	if err != nil {
		fmt.Println("failed to validate request. " + err.Error())
		return
	}

	bodyAfterValidation, _ := ioutil.ReadAll(httpReq.Body)

	raw := map[string]interface{}{}
	json.Unmarshal(bodyAfterValidation, &raw)

	if raw["testWithdefault"] != nil {
		fmt.Println("default value must not be included. ")
		return
	}
}

Thanks in advance,

[The-Murdock]

@fenollp adding default values into the request sends a lot of data over the wire which is not required.
The default values must be assumed on the receiver side

[fenollp]

Could you share the code you're using that triggers that http2 error? I'm curious how you're getting a header error when that ValidateRequest call is done as the request is received by the HTTP handler. These added defaults should not be sent over the wire.

Also, I'd suggest to open pull requests whenever you want to prove something with code instead of pasting non-colored diffs.

[orensolo]

@fenollp We are using ValidateRequest on the client side. The request is created before the validation. Then the validation adds the defaults values . But the content length in the header is the same as before the validation.

[fenollp]

I see. So we can either update the content length or add a validation option that disables checking defaults. I believe the latter is what you're aiming for?

[orensolo]

Yes the latter. You can use my solution above:

In options.go I added a new option (SkipDefaultValueSet) :

type Options struct {
	// Set ExcludeRequestBody so ValidateRequest skips request body validation
	ExcludeRequestBody bool

	// Set ExcludeResponseBody so ValidateResponse skips response body validation
	ExcludeResponseBody bool

	// Set IncludeResponseStatus so ValidateResponse fails on response
	// status not defined in OpenAPI spec
	IncludeResponseStatus bool

	MultiError bool

	// See NoopAuthenticationFunc
	AuthenticationFunc AuthenticationFunc
	//SkipDefaultValueSet indicates whether to skip setting default values
	// in the original request body
	SkipDefaultValueSet bool // <=== here is the new option
}

In ValidateRequestBody function in validate_request.go, please add the following code:

if !options.SkipDefaultValueSet {
	opts = append(opts, openapi3.DefaultsSet(func() { defaultsSet = true }))
}

instead of

opts = append(opts, openapi3.DefaultsSet(func() { defaultsSet = true }))

[orensolo]

@fenollp Please take a look at #662 and merge if all OK.
This pull request adds the option to skip adding default values to the request (but the default behavior is to add them)
Thanks!