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

Enum string value "YES" becomes "true" #1205

Open
piebur opened this issue Sep 25, 2019 · 8 comments
Open

Enum string value "YES" becomes "true" #1205

piebur opened this issue Sep 25, 2019 · 8 comments

Comments

@piebur
Copy link

piebur commented Sep 25, 2019

I have encountered an exception which looks suspicious while utilizing the String enum property with the value YES. The result is that YES becomes true somehow?

My guess is that is has to do with the conversion from YAML to JSON.

Reproduce:

  1. Unzip the attached file
  2. Copy the new-issue.yaml to the following location: swagger-parser\modules\swagger-parser-v3\src\test\resources
  3. Replace OpenAPIV3ParserTest.java in the following location: swagger-parser\modules\swagger-parser-v3\src\test\java\io\swagger\v3\parser\test
  4. Execute the test case testNewIssue(), the result is failure.

A work around could be to replace YES with some other string else or just add " to make it a string

Reference: vert-x3/vertx-web#1394

reproduce.zip

@webron
Copy link
Contributor

webron commented Sep 26, 2019

Yup, that's possible behavior when using YAML 1.1. YES can be considered as true (also ON and some other things). We should disable this kind of parsing though, and I was sure we did before. Perhaps it got 'lost' in the new version of the parser. Which version do you use?

As mentioned in the linked ticket, for now, the workaround is to explicitly say it's a string to avoid the parsing to consider it a boolean.

@piebur
Copy link
Author

piebur commented Sep 26, 2019

I am not sure what YAML version OpenApi uses, but I have within the new-issues.yaml set it to use OpenApi 3.0. Looking at the code OpenAPIV3Parser.java converts the YAML using io.swagger.v3.parser.OpenAPIV3Parser.YAML_MAPPER.
Yes the workaround is possible to use but looking at the yaml file it states that the enum values are of type String, so I think it should treat it accordingly.

This bug might be releated: OpenAPITools/openapi-generator#3196

@webron
Copy link
Contributor

webron commented Sep 26, 2019

That's actually not true, @piebur. JSON Schema doesn't work that way. The type has no impact on the enum or vice versa. You can define the type to be integer, and then have the enum contain only string values. The definition is legal and only means nothing will validate against it (except for no value, if that's allowed). An enum can also have multiple types in it. In short, we can't infer the type of the value in the enum from the defined type.

@manojkva
Copy link

I could fix it using the On/Off enum (the ones with issue) value to be under a double quote ("). Example
enum:

  • "On"
  • "Off"

@hkosova
Copy link

hkosova commented Sep 28, 2021

Related: swagger-api/swagger-core#3372

@davidmoten
Copy link

still happening in swagger-parser 2.1.16. Can this be fixed please?

@davidmoten
Copy link

Yup, that's possible behavior when using YAML 1.1. YES can be considered as true (also ON and some other things). We should disable this kind of parsing though, and I was sure we did before. Perhaps it got 'lost' in the new version of the parser. Which version do you use?

As mentioned in the linked ticket, for now, the workaround is to explicitly say it's a string to avoid the parsing to consider it a boolean.

That's not a practical workaround when I'm using external arbitrary openapi definitions. Is there a workaround that tells the swagger openapi parser to not make this substitution? I see this in the OpenAPI 3.0.3 specification:

In order to preserve the ability to round-trip between YAML and JSON formats, YAML version 1.2 is RECOMMENDED along with some additional constraints:

I've had a bit of a hunt in OpenAPIV3Parser class and I can see that I could make a copy of it and modify the YAML_FACTORY static field or I could modify it using reflection. Both ugly solutions, assuming that the YAMLFactory.configure(YAMLGenerator.Feature.MINIMIZE_QUOTES, false) is helpful.

I see some mention that Jackson 3.x will provide YAML 1.2 parsing. Are we waiting for that before fixing this issue? BTW I have no idea when Jackson 3.x will turn up.

See also #1741.

Just to give an idea of how much of this enum problem is out there in major org apis, the below list of 108 apis that use enums with one or more of non-quoted on, off, yes, no was garnered via a non-comprehensive search expression from https://github.com/APIs-guru/openapi-directory which has about 2000 apis. The consequence is that code generation from these apis that uses swagger-parser will be wrong or incomplete.

./APIs/adyen.com/CheckoutService/37/openapi.yaml
./APIs/adyen.com/CheckoutService/40/openapi.yaml
./APIs/adyen.com/CheckoutService/41/openapi.yaml
./APIs/adyen.com/CheckoutService/46/openapi.yaml
./APIs/adyen.com/CheckoutService/49/openapi.yaml
./APIs/adyen.com/CheckoutService/50/openapi.yaml
./APIs/adyen.com/CheckoutService/51/openapi.yaml
./APIs/adyen.com/CheckoutService/52/openapi.yaml
./APIs/adyen.com/CheckoutService/53/openapi.yaml
./APIs/adyen.com/CheckoutService/64/openapi.yaml
./APIs/adyen.com/CheckoutService/65/openapi.yaml
./APIs/adyen.com/CheckoutService/66/openapi.yaml
./APIs/adyen.com/CheckoutService/67/openapi.yaml
./APIs/adyen.com/CheckoutService/68/openapi.yaml
./APIs/adyen.com/CheckoutService/69/openapi.yaml
./APIs/adyen.com/CheckoutService/70/openapi.yaml
./APIs/adyen.com/PaymentService/25/openapi.yaml
./APIs/adyen.com/PaymentService/30/openapi.yaml
./APIs/adyen.com/PaymentService/40/openapi.yaml
./APIs/adyen.com/PaymentService/46/openapi.yaml
./APIs/adyen.com/PaymentService/49/openapi.yaml
./APIs/adyen.com/PaymentService/50/openapi.yaml
./APIs/adyen.com/PaymentService/51/openapi.yaml
./APIs/adyen.com/PaymentService/52/openapi.yaml
./APIs/adyen.com/PaymentService/64/openapi.yaml
./APIs/adyen.com/PaymentService/67/openapi.yaml
./APIs/adyen.com/PaymentService/68/openapi.yaml
./APIs/agco-ats.com/v1/openapi.yaml
./APIs/atlassian.com/jira/1001.0.0-SNAPSHOT/openapi.yaml
./APIs/billingo.hu/3.0.7/openapi.yaml
./APIs/canada-holidays.ca/1.8.0/openapi.yaml
./APIs/daniweb.com/4/openapi.yaml
./APIs/digitalocean.com/2.0/openapi.yaml
./APIs/drchrono.com/v4 (Hunt Valley)/openapi.yaml
./APIs/gerermesaffaires.com/1.0.6/openapi.yaml
./APIs/github.com/api.github.com/1.1.4/openapi.yaml
./APIs/github.com/api.github.com.2022-11-28/1.1.4/openapi.yaml
./APIs/github.com/ghec/1.1.4/openapi.yaml
./APIs/github.com/ghec.2022-11-28/1.1.4/openapi.yaml
./APIs/github.com/ghes-3.7/1.1.4/openapi.yaml
./APIs/github.com/ghes-3.8/1.1.4/openapi.yaml
./APIs/github.com/github.ae/1.1.4/openapi.yaml
./APIs/googleapis.com/abusiveexperiencereport/v1/openapi.yaml
./APIs/googleapis.com/adexperiencereport/v1/openapi.yaml
./APIs/googleapis.com/androidmanagement/v1/openapi.yaml
./APIs/googleapis.com/apigee/v1/openapi.yaml
./APIs/googleapis.com/appengine/v1alpha/openapi.yaml
./APIs/googleapis.com/appengine/v1beta/openapi.yaml
./APIs/googleapis.com/appengine/v1/openapi.yaml
./APIs/googleapis.com/bigquery/v2/openapi.yaml
./APIs/googleapis.com/cloudsearch/v1/openapi.yaml
./APIs/googleapis.com/compute/alpha/openapi.yaml
./APIs/googleapis.com/compute/beta/openapi.yaml
./APIs/googleapis.com/compute/v1/openapi.yaml
./APIs/googleapis.com/contentwarehouse/v1/openapi.yaml
./APIs/googleapis.com/customsearch/v1/openapi.yaml
./APIs/googleapis.com/dataproc/v1/openapi.yaml
./APIs/googleapis.com/dns/v1beta2/openapi.yaml
./APIs/googleapis.com/dns/v1/openapi.yaml
./APIs/googleapis.com/dns/v2/openapi.yaml
./APIs/googleapis.com/firebaseappcheck/v1beta/openapi.yaml
./APIs/googleapis.com/firebaseappcheck/v1/openapi.yaml
./APIs/googleapis.com/identitytoolkit/v2/openapi.yaml
./APIs/googleapis.com/integrations/v1alpha/openapi.yaml
./APIs/googleapis.com/integrations/v1/openapi.yaml
./APIs/googleapis.com/remotebuildexecution/v1alpha/openapi.yaml
./APIs/googleapis.com/vmmigration/v1alpha1/openapi.yaml
./APIs/googleapis.com/vmmigration/v1/openapi.yaml
./APIs/gov.bc.ca/geocoder/2.0.0/openapi.yaml
./APIs/gsmtasks.com/2.4.13/openapi.yaml
./APIs/hetzner.cloud/1.0.0/openapi.yaml
./APIs/just-eat.co.uk/1.0.0/openapi.yaml
./APIs/linode.com/4.151.1/openapi.yaml
./APIs/mermade.org.uk/openapi-converter/1.0.0/openapi.yaml
./APIs/microsoft.com/graph-beta/1.0.1/openapi.yaml
./APIs/netatmo.net/1.1.5/openapi.yaml
./APIs/nordigen.com/2.0 (v2)/openapi.yaml
./APIs/openbanking.org.uk/payment-initiation-openapi/3.1.7/openapi.yaml
./APIs/openbanking.org.uk/v1.3/openapi.yaml
./APIs/openfigi.com/1.4.0/openapi.yaml
./APIs/optimade.local/1.1.0~develop/openapi.yaml
./APIs/ote-godaddy.com/certificates/1.0.0/openapi.yaml
./APIs/ote-godaddy.com/domains/1.0.0/openapi.yaml
./APIs/ote-godaddy.com/orders/1.0.0/openapi.yaml
./APIs/pandascore.co/2.23.1/openapi.yaml
./APIs/plaid.com/2020-09-14_1.345.1/openapi.yaml
./APIs/probely.com/1.2.0/openapi.yaml
./APIs/rebilly.com/2.1/openapi.yaml
./APIs/rev.ai/v1/openapi.yaml
./APIs/reverb.com/3.0/openapi.yaml
./APIs/rubrikinc.github.io/v1/openapi.yaml
./APIs/shipengine.com/1.1.202304191404/openapi.yaml
./APIs/shutterstock.com/1.1.32/openapi.yaml
./APIs/squareup.com/2.0/openapi.yaml
./APIs/storecove.com/2.0.1/openapi.yaml
./APIs/stream-io-api.com/v80.2.0/openapi.yaml
./APIs/stripe.com/2022-11-15/openapi.yaml
./APIs/surevoip.co.uk/9dcb0dc8/openapi.yaml
./APIs/thebluealliance.com/3.8.2/openapi.yaml
./APIs/threatjammer.com/1.2.27/openapi.yaml
./APIs/ticketmaster.com/discovery/v2/openapi.yaml
./APIs/truora.com/1.0.0/openapi.yaml
./APIs/twilio.com/twilio_insights_v1/1.50.0/openapi.yaml
./APIs/vercel.com/0.0.1/openapi.yaml
./APIs/xero.com/xero_accounting/2.9.4/openapi.yaml
./APIs/xero.com/xero_bankfeeds/2.9.4/openapi.yaml
./APIs/zoom.us/2.0.0/openapi.yaml
./APIs/zuora.com/2021-08-20/openapi.yaml

@davidmoten
Copy link

I came up with a workaround. I create a subclass of OpenAPIV3Parser called EnhancedOpenAPIV3Parser. This new class reads the OpenAPI definition file into a string as before but then uses the YAML 1.2 compliant snakeyaml-engine artifact to read the string and convert it to use double quotes (and !! typing). Parsing continues as before but no annoying yes, no, on, off replacement with true, false.

https://github.com/davidmoten/openapi-codegen/blob/458ebda3e6bbf2c67f0cfefedcf477296bfd5b67/openapi-codegen-generator/src/main/java/org/davidmoten/oa3/codegen/generator/internal/EnhancedOpenAPIV3Parser.java

OpenAPIV3Parser parser = new EnhancedOpenAPIV3Parser();
SwaggerParseResult result = parser.readLocation(definition, null, options);

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

No branches or pull requests

5 participants