search

swagger-api / swagger-parser

Swagger Spec to Java POJOs
http://swagger.io
Apache License 2.0
778 stars 525 forks source link

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

Open piebur opened 4 years ago

piebur commented 4 years ago

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: https://github.com/vert-x3/vertx-web/issues/1394

reproduce.zip

webron commented 4 years ago

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 commented 4 years ago

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: https://github.com/OpenAPITools/openapi-generator/issues/3196

webron commented 4 years ago

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 commented 4 years ago

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

hkosova commented 2 years ago

Related: https://github.com/swagger-api/swagger-core/issues/3372

davidmoten commented 11 months ago

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

davidmoten commented 11 months ago

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 https://github.com/swagger-api/swagger-parser/issues/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 commented 11 months ago

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);