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

Don't validate readonly field for write requests or vice-versa #532

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Don't validate readonly field for write requests or vice-versa #532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8550617
Don't validate readonly field for write requests
CTrando Apr 3, 2022
80f743e
add back check
CTrando Apr 3, 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
22 changes: 13 additions & 9 deletions openapi3/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,11 @@ func (schema *Schema) WithAdditionalProperties(v *Schema) *Schema {
return schema
}

func (schema *Schema) IsEmpty() bool {
func (schema *Schema) IsEmpty(settings *schemaValidationSettings) bool {
if (schema.ReadOnly && settings.writeEp) || (schema.WriteOnly && settings.readEp) {
return true
}

if schema.Type != "" || schema.Format != "" || len(schema.Enum) != 0 ||
schema.UniqueItems || schema.ExclusiveMin || schema.ExclusiveMax ||
schema.Nullable || schema.ReadOnly || schema.WriteOnly || schema.AllowEmptyValue ||
Expand All @@ -558,35 +562,35 @@ func (schema *Schema) IsEmpty() bool {
schema.MinProps != 0 || schema.MaxProps != nil {
return false
}
if n := schema.Not; n != nil && !n.Value.IsEmpty() {
if n := schema.Not; n != nil && !n.Value.IsEmpty(settings) {
return false
}
if ap := schema.AdditionalProperties; ap != nil && !ap.Value.IsEmpty() {
if ap := schema.AdditionalProperties; ap != nil && !ap.Value.IsEmpty(settings) {
return false
}
if apa := schema.AdditionalPropertiesAllowed; apa != nil && !*apa {
return false
}
if items := schema.Items; items != nil && !items.Value.IsEmpty() {
if items := schema.Items; items != nil && !items.Value.IsEmpty(settings) {
return false
}
for _, s := range schema.Properties {
if !s.Value.IsEmpty() {
if !s.Value.IsEmpty(settings) {
return false
}
}
for _, s := range schema.OneOf {
if !s.Value.IsEmpty() {
if !s.Value.IsEmpty(settings) {
return false
}
}
for _, s := range schema.AnyOf {
if !s.Value.IsEmpty() {
if !s.Value.IsEmpty(settings) {
return false
}
}
for _, s := range schema.AllOf {
if !s.Value.IsEmpty() {
if !s.Value.IsEmpty(settings) {
return false
}
}
Expand Down Expand Up @@ -797,7 +801,7 @@ func (schema *Schema) visitJSON(settings *schemaValidationSettings, value interf
}
}

if schema.IsEmpty() {
if schema.IsEmpty(settings) {
return
}
if err = schema.visitSetOperations(settings, value); err != nil {
Expand Down
16 changes: 13 additions & 3 deletions openapi3/schema_validation_settings.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ package openapi3
type SchemaValidationOption func(*schemaValidationSettings)

type schemaValidationSettings struct {
failfast bool
multiError bool
asreq, asrep bool // exclusive (XOR) fields
failfast bool
multiError bool
asreq, asrep bool // exclusive (XOR) fields
writeEp, readEp bool
}

// FailFast returns schema validation errors quicker.
Expand All @@ -21,10 +22,19 @@ func MultiErrors() SchemaValidationOption {
func VisitAsRequest() SchemaValidationOption {
return func(s *schemaValidationSettings) { s.asreq, s.asrep = true, false }
}

func VisitAsResponse() SchemaValidationOption {
return func(s *schemaValidationSettings) { s.asreq, s.asrep = false, true }
}

func ReadEndpoint() SchemaValidationOption {
return func(s *schemaValidationSettings) { s.writeEp, s.readEp = false, true }
}

func WriteEndpoint() SchemaValidationOption {
return func(s *schemaValidationSettings) { s.readEp, s.writeEp = false, true }
}

func newSchemaValidationSettings(opts ...SchemaValidationOption) *schemaValidationSettings {
settings := &schemaValidationSettings{}
for _, opt := range opts {
Expand Down
3 changes: 3 additions & 0 deletions openapi3filter/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,7 @@ type Options struct {

// See NoopAuthenticationFunc
AuthenticationFunc AuthenticationFunc

// See EndpointType
EndpointType EndpointType
}
185 changes: 182 additions & 3 deletions openapi3filter/validate_readonly_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,184 @@ import (
"github.com/stretchr/testify/require"
)

func TestValidatingRequestBodyWithReadOnlyProperty(t *testing.T) {
func TestValidatingReadRequestBodyWithReadOnlyProperty(t *testing.T) {
const spec = `{
"openapi": "3.0.3",
"info": {
"version": "1.0.0",
"title": "title",
"description": "desc",
"contact": {
"email": "email"
}
},
"paths": {
"/accounts": {
"get": {
"description": "Get an account",
"requestBody": {
"required": true,
"content": {
"application/json": {
"schema": {
"type": "object",
"required": ["_id"],
"properties": {
"_id": {
"type": "string",
"description": "Unique identifier for this object.",
"pattern": "[0-9a-v]+$",
"minLength": 20,
"maxLength": 20,
"readOnly": true
}
}
}
}
}
},
"responses": {
"201": {
"description": "Successfully got an account"
},
"400": {
"description": "The server could not understand the request due to invalid syntax",
}
}
}
}
}
}
`

type Request struct {
ID string `json:"_id"`
}

sl := openapi3.NewLoader()
doc, err := sl.LoadFromData([]byte(spec))
require.NoError(t, err)
err = doc.Validate(sl.Context)
require.NoError(t, err)
router, err := legacyrouter.NewRouter(doc)
require.NoError(t, err)

b, err := json.Marshal(Request{ID: "bt6kdc3d0cvp6u8u3ft0"})
require.NoError(t, err)

httpReq, err := http.NewRequest(http.MethodGet, "/accounts", bytes.NewReader(b))
require.NoError(t, err)
httpReq.Header.Add(headerCT, "application/json")

route, pathParams, err := router.FindRoute(httpReq)
require.NoError(t, err)

err = ValidateRequest(sl.Context, &RequestValidationInput{
Request: httpReq,
PathParams: pathParams,
Route: route,
Options: &Options{EndpointType: ReadEndpoint},
})
require.NoError(t, err)

// Try again with an insufficient length ID
b, err = json.Marshal(Request{ID: "0cvp6u8u3ft0"})
require.NoError(t, err)

httpReq, err = http.NewRequest(http.MethodGet, "/accounts", bytes.NewReader(b))
require.NoError(t, err)
httpReq.Header.Add(headerCT, "application/json")

err = ValidateRequest(sl.Context, &RequestValidationInput{
Request: httpReq,
PathParams: pathParams,
Route: route,
Options: &Options{EndpointType: ReadEndpoint},
})
require.Error(t, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

wait, why would this fail due to "insufficient length ID" if this is a read endpoint and a read-only field (i.e., it shouldn't be validated)?

}

func TestValidatingIfReadOrWriteEndpointIsNotKnownDefaultValidatesProperty(t *testing.T) {
const spec = `{
"openapi": "3.0.3",
"info": {
"version": "1.0.0",
"title": "title",
"description": "desc",
"contact": {
"email": "email"
}
},
"paths": {
"/accounts": {
"post": {
"description": "Get an account",
"requestBody": {
"required": true,
"content": {
"application/json": {
"schema": {
"type": "object",
"required": ["_id"],
"properties": {
"_id": {
"type": "string",
"description": "Unique identifier for this object.",
"pattern": "[0-9a-v]+$",
"minLength": 20,
"maxLength": 20,
"readOnly": true
}
}
}
}
}
},
"responses": {
"201": {
"description": "Successfully got an account"
},
"400": {
"description": "The server could not understand the request due to invalid syntax",
}
}
}
}
}
}
`

type Request struct {
ID string `json:"_id"`
}

sl := openapi3.NewLoader()
doc, err := sl.LoadFromData([]byte(spec))
require.NoError(t, err)
err = doc.Validate(sl.Context)
require.NoError(t, err)
router, err := legacyrouter.NewRouter(doc)
require.NoError(t, err)

b, err := json.Marshal(Request{ID: ""})
require.NoError(t, err)

httpReq, err := http.NewRequest(http.MethodPost, "/accounts", bytes.NewReader(b))
require.NoError(t, err)
httpReq.Header.Add(headerCT, "application/json")

route, pathParams, err := router.FindRoute(httpReq)
require.NoError(t, err)

err = ValidateRequest(sl.Context, &RequestValidationInput{
Request: httpReq,
PathParams: pathParams,
Route: route,
})
require.Error(t, err)
}

func TestValidatingWriteRequestOnRequiredReadOnlyProperty(t *testing.T) {
const spec = `{
"openapi": "3.0.3",
"info": {
Expand Down Expand Up @@ -49,7 +226,7 @@ func TestValidatingRequestBodyWithReadOnlyProperty(t *testing.T) {
},
"responses": {
"201": {
"description": "Successfully created a new account"
"description": "Successfully created an account"
},
"400": {
"description": "The server could not understand the request due to invalid syntax",
Expand All @@ -73,7 +250,8 @@ func TestValidatingRequestBodyWithReadOnlyProperty(t *testing.T) {
router, err := legacyrouter.NewRouter(doc)
require.NoError(t, err)

b, err := json.Marshal(Request{ID: "bt6kdc3d0cvp6u8u3ft0"})
// Set no id because id is a required readonly field, but this is a write request
b, err := json.Marshal(Request{ID: ""})
require.NoError(t, err)

httpReq, err := http.NewRequest(http.MethodPost, "/accounts", bytes.NewReader(b))
Expand All @@ -87,6 +265,7 @@ func TestValidatingRequestBodyWithReadOnlyProperty(t *testing.T) {
Request: httpReq,
PathParams: pathParams,
Route: route,
Options: &Options{EndpointType: WriteEndpoint},
})
require.NoError(t, err)
}
6 changes: 6 additions & 0 deletions openapi3filter/validate_request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,12 @@ func ValidateRequestBody(ctx context.Context, input *RequestValidationInput, req
if options.MultiError {
opts = append(opts, openapi3.MultiErrors())
}
if options.EndpointType == ReadEndpoint {
opts = append(opts, openapi3.ReadEndpoint())
}
if options.EndpointType == WriteEndpoint {
opts = append(opts, openapi3.WriteEndpoint())
}

// Validate JSON with the schema
if err := contentType.Schema.Value.VisitJSON(value, opts...); err != nil {
Expand Down
Loading