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

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

Open
griffio opened this issue Feb 1, 2019 · 4 comments
Open

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

griffio opened this issue Feb 1, 2019 · 4 comments
Labels
bug Unexpected behaviour for functionality that either was intended to work, or has worked in the past help wanted Easy to moderately difficult issues that don't require deep knowledge or architectural thought scala-akka-http Pertains to guardrail-scala-akka-http

Comments

@griffio
Copy link
Contributor

griffio commented Feb 1, 2019

💂‍♀️🚃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
Copy link
Member

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
Copy link
Contributor Author

griffio commented Feb 2, 2019

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
Copy link
Contributor

dsilvasc commented Apr 1, 2019

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
Copy link
Member

@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.

@blast-hardcheese blast-hardcheese added bug Unexpected behaviour for functionality that either was intended to work, or has worked in the past help wanted Easy to moderately difficult issues that don't require deep knowledge or architectural thought scala-akka-http Pertains to guardrail-scala-akka-http labels Apr 2, 2019
blast-hardcheese added a commit to blast-hardcheese/guardrail that referenced this issue Apr 23, 2019
blast-hardcheese added a commit to blast-hardcheese/guardrail that referenced this issue May 11, 2019
This was referenced May 11, 2019
blast-hardcheese added a commit to blast-hardcheese/guardrail that referenced this issue May 13, 2019
blast-hardcheese added a commit to blast-hardcheese/guardrail that referenced this issue Jun 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected behaviour for functionality that either was intended to work, or has worked in the past help wanted Easy to moderately difficult issues that don't require deep knowledge or architectural thought scala-akka-http Pertains to guardrail-scala-akka-http
Projects
None yet
Development

No branches or pull requests

3 participants