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

fix(parameters): applied extra schema validation to parameters #166

Merged

Conversation

fredbi
Copy link
Member

@fredbi fredbi commented Dec 23, 2023

This PR applies an extra jsonschema validation to expanded parameters.

Whenever a parameter is provided as a $ref, jsonschema validation works at the level of the unexpanded parameter only. It is indeed valid to declare a parameter as a json reference. The issue is that the contents of this parameter definition are subject only to "extra rules" (uniqueness, etc) and not the basic jsonschema rules (e.g. required properties).

An example is provided by go-swagger/go-swagger#2527.

It seems that this situation never occured before go-swagger/go-swagger#2527 because the "parameters" section to which shared parameters normally point to is always fully expanded.

In the case of a $ref to a schema in "definition", the invalid content of the $ref incorrectly passes validation.

Copy link

codecov bot commented Dec 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (dd1b7cd) 91.47% compared to head (94f064e) 92.40%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #166      +/-   ##
==========================================
+ Coverage   91.47%   92.40%   +0.92%     
==========================================
  Files          22       22              
  Lines        2978     2987       +9     
==========================================
+ Hits         2724     2760      +36     
+ Misses        202      182      -20     
+ Partials       52       45       -7     
Flag Coverage Δ
oldstable 92.33% <100.00%> (+0.92%) ⬆️
stable 92.33% <100.00%> (+0.86%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

fredbi added a commit to fredbi/go-swagger that referenced this pull request Dec 23, 2023
* fixes go-swagger#2527
* requires go-openapi/validate#166

TODO: add unit tests

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
@fredbi fredbi force-pushed the fix/165-extra-schema-validation-for-params branch from 2942f19 to 7ca958e Compare December 24, 2023 17:29
fredbi added a commit to fredbi/go-swagger that referenced this pull request Dec 24, 2023
* fixes go-swagger#2527
* requires go-openapi/validate#166

TODO: add unit tests

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
@fredbi fredbi marked this pull request as ready for review December 25, 2023 11:55
spec.go Show resolved Hide resolved
This PR applies an extra jsonschema validation to expanded parameters.

Whenever a parameter is provided as a $ref, jsonschema validation works at the level of the unexpanded parameter only.
It is indeed valid to declare a parameter as a json reference.

The issue is that the expanded content of this parameter definition is subject only to "extra rules" (uniqueness, etc)
and not to the basic jsonschema rules (e.g. allowed and required properties).

An example is provided by go-swagger/go-swagger#2527.

It seems that this situation never occured before go-swagger/go-swagger#2527 because the "parameters" section to which shared
parameters normally point to is always fully expanded.

In the case of a $ref to a schema in "definition", the invalid content of the $ref incorrectly passes validation.

* fixes go-openapi#165
* contributes go-swagger/go-swagger#2527

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
@fredbi fredbi force-pushed the fix/165-extra-schema-validation-for-params branch from 7ca958e to 94f064e Compare December 27, 2023 10:10
@fredbi fredbi changed the title fix(parameters): appled extra schema validation to parameters fix(parameters): applied extra schema validation to parameters Dec 27, 2023
@fredbi fredbi merged commit 1d1d938 into go-openapi:master Dec 27, 2023
10 checks passed
@fredbi fredbi deleted the fix/165-extra-schema-validation-for-params branch December 27, 2023 10:16
fredbi added a commit to fredbi/go-swagger that referenced this pull request Dec 27, 2023
* Added unit tests to assert that invalid parameters in spec
are indeed detected (i) by spec validation, (ii) by codegen when
validation is skipped.
* Refactored a bit client codegen tests

* fixes go-swagger#2527
* requires go-openapi/validate#166

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
fredbi added a commit to fredbi/go-swagger that referenced this pull request Dec 27, 2023
* Added unit tests to assert that invalid parameters in spec
are indeed detected (i) by spec validation, (ii) by codegen when
validation is skipped.
* Refactored a bit client codegen tests

* fixes go-swagger#2527
* requires go-openapi/validate#166

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
fredbi added a commit to go-swagger/go-swagger that referenced this pull request Dec 27, 2023
* Added unit tests to assert that invalid parameters in spec
are indeed detected (i) by spec validation, (ii) by codegen when
validation is skipped.
* Refactored a bit client codegen tests

* fixes #2527
* requires go-openapi/validate#166

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

parameters provided a $ref skip the jsonschema validation
2 participants