guardrail-dev / guardrail

Principled code generation from OpenAPI specifications
https://guardrail.dev
MIT License
524 stars 132 forks source link

Parameters of "type: string" reject numeric values as "malformed" #184

Open griffio opened 5 years ago

griffio commented 5 years ago

๐Ÿ’‚โ€โ™€๏ธ๐ŸšƒWe are sending query parameters that can be either a number or alpha-numeric input

In the latest version and earlier. The following simple fragment when generated as a route will only allow alpha-numeric values to be passed in the request.

For example: using a numeric value is rejected as "The query parameter 'a' was malformed: String"

  /foo:
    get:
      operationId: getFoo
      x-scala-package: foo
      produces:
        - application/json
      parameters:
        - name: a
          in: query
          type: string      

๐Ÿ™‰Hope that makes sense that we may expect the decoder to allow numerics to be converted to a string rather than be rejected.

blast-hardcheese commented 5 years ago

Yikes, yeah, that's not great; as a workaround, if you have a Circe decoder for Either [A, B], a workaround would be to x-scala-type: Either[String, Long].

griffio commented 5 years ago

The parameter is turned into a ๐Ÿ”ขJSNumber by the J.decodeJson(json) and sends it into a decodeString: Decoder[String] circe thing. ๐Ÿ‘โ€๐Ÿ—จhttps://github.com/twilio/guardrail/blob/c476988b5bf3024bde0fa91c73580b0c441842e1/modules/codegen/src/main/scala/com/twilio/guardrail/generators/AkkaHttpGenerator.scala#L132

dsilvasc commented 5 years ago

Is the advice here then to annotate every swagger string input parameter as x-scala-type: Either[String, Long] in case the input characters are all digits?

blast-hardcheese commented 5 years ago

@dsilvasc I honestly haven't had a chance to circle around and fix the underlying bug here. I'd rather this not be the case, but for now I can't offer a better solution.

What needs to happen is the underlying types from swagger need to be reflected in akka-http's directives (as[Long] or equivalent) before being wrapped (currently all parameters are wrapped with circe) and decoded into domain-specific types (again, currently done via circe).

Right now, we're just taking everything as a string, attempting to parse it as JSON, then treating it as a string before proceeding. This creates an unfortunate situation where if a valid JSON symbol (numbers, boolean literals) are supplied, they'll get parsed as those literals, failing the string decoder. This has the additional unfortunate impact of removing surrounding double quotes from strings, as they're parsed as JSON string literals.