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.

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

UUID string format validation too lax #147

Open
plore opened this issue Mar 12, 2023 · 2 comments · May be fixed by #149
Open

UUID string format validation too lax #147

plore opened this issue Mar 12, 2023 · 2 comments · May be fixed by #149
Labels
ready ready to implement

Comments

@plore
Copy link

plore commented Mar 12, 2023

Hi,

first of all, thanks for all your efforts on this project!

I noticed that string validation for the uuid format is a bit too lax and passes on obviously non-uuid strings:

Given the following OpenAPI file

#schema.yml
openapi: 3.0.3
paths:
  /foo:
    get:
      parameters:
        - name: id
          in: query
          schema:
            type: string
            format: uuid

I would expect the following code to fail

require 'openapi_parser'
require 'yaml'

root = OpenAPIParser.parse(YAML.load_file('schema.yml'))
request_operation = root.request_operation(:get, '/foo')

request_operation.validate_request_parameter({"id" => "204730df-d3f5-364b-9aeb-d1372aba0d35-foobar"}, {})
request_operation.validate_request_parameter({"id" => "foobar-204730df-d3f5-364b-9aeb-d1372aba0d35"}, {})

But it runs without error.
I guess your regex matches as long as it finds a valid uuid substring.

Any chance of tightening this up? Would be willing to help if you think it a good idea.

@ota42y
Copy link
Owner

ota42y commented Mar 12, 2023

Yes, we should be fix this problem!
This is bug because we check UUID format and this string pass checking 🤔
https://github.com/ota42y/openapi_parser/blob/master/lib/openapi_parser/schema_validator/string_validator.rb#L69

@ota42y ota42y added the ready ready to implement label Mar 12, 2023
@plore plore linked a pull request Mar 19, 2023 that will close this issue
@plore
Copy link
Author

plore commented Jan 19, 2024

@ota42y #149 should fix this problem. Is anything else blocking this or could we move forward?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready ready to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants