Repository: getkin/kin-openapi
Pull request #200 (2020-04-02 09:14:58)

fix panic when multipart form contains extra parts not present in schema
Validating a request body with content 'multipart/form-data' that contained parts not defined in the schema caused Panic, 
fixed by ignoring the undefined properties

fenollp: Nice catch. Why should these be silently ignored? Shouldn't an error be returned here?
Ventrosky: @fenollp well if somebody is migrating API version and have some app that still has a form which  uses extra form fields we might not want to break everything and let them handle extra values, that's what I thought at least, returning an error is a good option too
fenollp: Is it not possible to also define the old form schema and wrap the two in a `oneOf`?
Or maybe un-schema'd data should be ignored as long as `additionalProperties: false` is not in the schema? otherwise an error should be returned. 

I'd like kin-openapi to be as strict as can be :)
I think there should be a check on additionalProperties. Could that work for your use case?
Ventrosky: Yes, you have a point. Sure works for me :) 
Ventrosky: I have added an extra commit trying to implement the proposed solution, thanks for the quick response btw :)
Ventrosky: Additional properties that are defined in the schema, for example
```
additionalProperties:
  type: object
  properties:
    code:
      type: integer
```
 those might have to be included in the decoded body form and only ignore only those undefined
```
additionalProperties: true
```
fenollp: LGTM. Could you add yet another (thanks!) that explicitly checks for error on `additionalProperties: false`?
Thank you
Ventrosky: Sure thing, done, thank you so much for your work and responsiveness!

Patch:
diff --git a/openapi3filter/req_resp_decoder.go b/openapi3filter/req_resp_decoder.go
index 06ce223..f65b142 100644
--- a/openapi3filter/req_resp_decoder.go
+++ b/openapi3filter/req_resp_decoder.go
@@ -923,7 +923,29 @@ func multipartBodyDecoder(body io.Reader, header http.Header, schema *openapi3.S
 		subEncFn := func(string) *openapi3.Encoding { return enc }
 		// If the property's schema has type "array" it is means that the form contains a few parts with the same name.
 		// Every such part has a type that is defined by an items schema in the property's schema.
-		valueSchema := schema.Value.Properties[name]
+		var valueSchema *openapi3.SchemaRef
+		var exists bool
+		valueSchema, exists = schema.Value.Properties[name]
+		if !exists {
+			anyProperties := schema.Value.AdditionalPropertiesAllowed
+			if anyProperties != nil {
+				switch *anyProperties {
+				case true:
+					//additionalProperties: true
+					continue
+				default:
+					//additionalProperties: false
+					return nil, &ParseError{Kind: KindOther, Cause: fmt.Errorf("part %s: undefined", name)}
+				}
+			}
+			if schema.Value.AdditionalProperties == nil {
+				return nil, &ParseError{Kind: KindOther, Cause: fmt.Errorf("part %s: undefined", name)}
+			}
+			valueSchema, exists = schema.Value.AdditionalProperties.Value.Properties[name]
+			if !exists {
+				return nil, &ParseError{Kind: KindOther, Cause: fmt.Errorf("part %s: undefined", name)}
+			}
+		}
 		if valueSchema.Value.Type == "array" {
 			valueSchema = valueSchema.Value.Items
 		}
@@ -938,9 +960,18 @@ func multipartBodyDecoder(body io.Reader, header http.Header, schema *openapi3.S
 		values[name] = append(values[name], value)
 	}
 
+	allTheProperties := make(map[string]*openapi3.SchemaRef)
+	for k, v := range schema.Value.Properties {
+		allTheProperties[k] = v
+	}
+	if schema.Value.AdditionalProperties != nil {
+		for k, v := range schema.Value.AdditionalProperties.Value.Properties {
+			allTheProperties[k] = v
+		}
+	}
 	// Make an object value from form values.
 	obj := make(map[string]interface{})
-	for name, prop := range schema.Value.Properties {
+	for name, prop := range allTheProperties {
 		vv := values[name]
 		if len(vv) == 0 {
 			continue


Test patch:
diff --git a/openapi3filter/req_resp_decoder_test.go b/openapi3filter/req_resp_decoder_test.go
index 49fb3a1..4a5cefc 100644
--- a/openapi3filter/req_resp_decoder_test.go
+++ b/openapi3filter/req_resp_decoder_test.go
@@ -980,6 +980,27 @@ func TestDecodeBody(t *testing.T) {
 	})
 	require.NoError(t, err)
 
+	multipartFormExtraPart, multipartFormMimeExtraPart, err := newTestMultipartForm([]*testFormPart{
+		{name: "a", contentType: "text/plain", data: strings.NewReader("a1")},
+		{name: "x", contentType: "text/plain", data: strings.NewReader("x1")},
+	})
+	require.NoError(t, err)
+
+	multipartAnyAdditionalProps, multipartMimeAnyAdditionalProps, err := newTestMultipartForm([]*testFormPart{
+		{name: "a", contentType: "text/plain", data: strings.NewReader("a1")},
+		{name: "x", contentType: "text/plain", data: strings.NewReader("x1")},
+	})
+	multipartAdditionalProps, multipartMimeAdditionalProps, err := newTestMultipartForm([]*testFormPart{
+		{name: "a", contentType: "text/plain", data: strings.NewReader("a1")},
+		{name: "x", contentType: "text/plain", data: strings.NewReader("x1")},
+	})
+	multipartAdditionalPropsErr, multipartMimeAdditionalPropsErr, err := newTestMultipartForm([]*testFormPart{
+		{name: "a", contentType: "text/plain", data: strings.NewReader("a1")},
+		{name: "x", contentType: "text/plain", data: strings.NewReader("x1")},
+		{name: "y", contentType: "text/plain", data: strings.NewReader("y1")},
+	})
+	require.NoError(t, err)
+
 	testCases := []struct {
 		name     string
 		mime     string
@@ -1060,6 +1081,45 @@ func TestDecodeBody(t *testing.T) {
 				WithProperty("f", openapi3.NewStringSchema().WithFormat("binary")),
 			want: map[string]interface{}{"a": "a1", "b": float64(10), "c": []interface{}{"c1", "c2"}, "d": map[string]interface{}{"d1": "d1"}, "f": "foo"},
 		},
+		{
+			name: "multipartExtraPart",
+			mime: multipartFormMimeExtraPart,
+			body: multipartFormExtraPart,
+			schema: openapi3.NewObjectSchema().
+				WithProperty("a", openapi3.NewStringSchema()),
+			want:    map[string]interface{}{"a": "a1"},
+			wantErr: &ParseError{Kind: KindOther},
+		},
+		{
+			name: "multipartAnyAdditionalProperties",
+			mime: multipartMimeAnyAdditionalProps,
+			body: multipartAnyAdditionalProps,
+			schema: openapi3.NewObjectSchema().
+				WithAnyAdditionalProperties().
+				WithProperty("a", openapi3.NewStringSchema()),
+			want: map[string]interface{}{"a": "a1"},
+		},
+		{
+			name: "multipartWithAdditionalProperties",
+			mime: multipartMimeAdditionalProps,
+			body: multipartAdditionalProps,
+			schema: openapi3.NewObjectSchema().
+				WithAdditionalProperties(openapi3.NewObjectSchema().
+					WithProperty("x", openapi3.NewStringSchema())).
+				WithProperty("a", openapi3.NewStringSchema()),
+			want: map[string]interface{}{"a": "a1", "x": "x1"},
+		},
+		{
+			name: "multipartWithAdditionalPropertiesError",
+			mime: multipartMimeAdditionalPropsErr,
+			body: multipartAdditionalPropsErr,
+			schema: openapi3.NewObjectSchema().
+				WithAdditionalProperties(openapi3.NewObjectSchema().
+					WithProperty("x", openapi3.NewStringSchema())).
+				WithProperty("a", openapi3.NewStringSchema()),
+			want:    map[string]interface{}{"a": "a1", "x": "x1"},
+			wantErr: &ParseError{Kind: KindOther},
+		},
 		{
 			name: "file",
 			mime: "application/octet-stream",