Skip to content

Validation pass when object, list or integer returned instead of string for object string property (must fail) #105

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

Closed
andyceo opened this issue Nov 15, 2018 · 4 comments
Labels
area/validation/request Indicates an issue on request validation area. kind/bug/confirmed

Comments

@andyceo
Copy link

andyceo commented Nov 15, 2018

Hi!

We faced with following bug. Some object's property declared as string, If this object's property returned by API is object itself, or integer, or list, no errors generated.

If API returns wrong property type for example int property, InvalidMediaTypeValue thrown (as expected).

Tested on version 0.5.0, 0.7.1, master - same results.

Expected behavior

InvalidMediaType should be thrown

Steps to reproduce

  1. Install openapi-core:

     pip3 install openapi-core
    

2.. Create and save following fatless specification as test.yml:

openapi: "3.0.1"

info:
  version: "0.1"
  title: Object instead of string
  description: Test for if returns objects instead of string

components:
  schemas:
	SomeObject:
	  type: object
	  properties:
		someprop:
		  description: Some property
		  type: string
		someint:
		  type: integer

paths:
  /getsomeobject:
	get:
	  summary: Get the SomeObject
	  operationId: getSomeObject
	  responses:
		'200':
		  description: This is SomeObject
		  content:
			application/json:
			  schema:
				$ref: '#/components/schemas/SomeObject'
  1. Create and save following script as test.py:

     #!/usr/bin/env python3
     
     # -*- coding: utf-8 -*-
     import json
     import sys
     import yaml
     from openapi_core import create_spec
     from openapi_core.shortcuts import RequestValidator, ResponseValidator
     from openapi_core.wrappers.mock import MockRequest, MockResponse
     
     
     def validate(openapi_file):
         with open(openapi_file, 'r') as myfile:
             spec_dict = yaml.safe_load(myfile)
             spec = create_spec(spec_dict)
     
             openapi_request = MockRequest('localhost', 'get', '/getsomeobject')
             validator = RequestValidator(spec)
             result = validator.validate(openapi_request)
             request_errors = result.errors
     
             # PASS (must PASS)
             data = json.dumps({
                 'someprop': 'content'
             })
     
             # PASS (must FAIL)
             data = json.dumps({
                 'someprop': {
                     'nested_object_property': 'content',
                     'nested_object_another property': 13,
                 }
             })
     
             # PASS (must FAIL)
             data = json.dumps({
                 'someprop': ['dfdfdf', 'dfdfdfsssss']
             })
     
             # PASS (must FAIL)
             data = json.dumps({
                 'someprop': 123
             })
     
             # PASS (must FAIL)
             data = json.dumps({
                 'someprop': 123
             })
     
             # FAIL (must FAIL)
             data = json.dumps({
                 'someint': 'content'
             })
     
             # FAIL (must FAIL)
             data = json.dumps({
                 'someprop': 'dsdsd',
                 'someint': 123,
                 'not_in_scheme_prop': 123
             })
     
             openapi_response = MockResponse(data)
             validator = ResponseValidator(spec)
             result = validator.validate(openapi_request, openapi_response)
             response_errors = result.errors
     
             print('Request errors: {} Response errors: {}'.format(request_errors, response_errors))
     
     
     if __name__ == "__main__":
         if len(sys.argv) < 2:
             print("Specify path to openapi.yaml file!")
             exit(1)
         else:
             validate(sys.argv[1])
    
  2. Execute script to validate spec:

     python3 test.py test.yml
    
  3. Try to comment out test payloads to see actual results.

@andyceo andyceo changed the title Validation pass when object returned instead string (should fail) Validation pass when object, list or integer returned instead of string (must fail) for object string property Nov 15, 2018
@andyceo andyceo changed the title Validation pass when object, list or integer returned instead of string (must fail) for object string property Validation pass when object, list or integer returned instead of string for object string property (must fail) Nov 15, 2018
@andyceo
Copy link
Author

andyceo commented Nov 15, 2018

Initial report is here: ergoplatform/ergo#555

@p1c2u p1c2u added kind/bug/confirmed area/validation/request Indicates an issue on request validation area. labels Nov 15, 2018
@danielbradburn
Copy link
Contributor

I have recently started experimenting with openapi_core and also came across this issue. I think the problem is that when unmarshalling a string and no format is given str is used, which in python means lot's of things can correctly be unmarshalled as strings (including numbers, arrays, objects etc.)

I have worked around this issue for now by monkey patching so that I can continue experimenting with this really cool library (thanks for this by the way @p1c2u !)

@contextlib.contextmanager
def strict_str():
    """
    openapi_core unmarshals and validates strings by converting whatever
    is in the response to a str, and then validating that what we get is
    a str. This is of course means that lot's of things convert
    fine to a string. This means that we cannot validate string
    properties.

    To workaround this issue this function provides a means to patch
    openapi_core to use our own custom formatting for strings which
    strictly checks that a value is a string.

    For example...

      >>> with strict_str():
      ...   validator = ResponseValidator(...)
    """

    def strict_to_str(x):
        if not isinstance(x, str):
            raise OpenAPISchemaError(f"Expected str but got {type(x)} -- {x}")
        return x

    original = Schema.STRING_FORMAT_CALLABLE_GETTER
    patched = dict(original)
    patched[SchemaFormat.NONE] = Format(strict_to_str, lambda x: isinstance(x, str))

    target = "openapi_core.schema.schemas.models.Schema.STRING_FORMAT_CALLABLE_GETTER"
    with patch(target, patched):
        yield

@p1c2u what do you think about making the unmarshalling of strings more strict so that this patching is not needed? I would be happy to make a PR if you agree with the approach (basically when no format is given we use strict type checking to ensure we actually have a string)

@danielbradburn
Copy link
Contributor

Ah actually I see there is already a PR!

@andyceo
Copy link
Author

andyceo commented Mar 23, 2019

Oh thank you! I'll check this within 2-3 weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/validation/request Indicates an issue on request validation area. kind/bug/confirmed
Projects
None yet
Development

No branches or pull requests

4 participants
@andyceo @p1c2u @danielbradburn and others