ErikWittern / openapi-snippet

Generates code snippets for given Swagger / Open API documents
MIT License
115 stars 66 forks source link

Add x-www-form-urlencoded media type examples support #67

Closed d-silva closed 3 years ago

d-silva commented 3 years ago

Fix issue #66

d-silva commented 3 years ago

Thanks for your fast reply!

About the dependency on querystring, maybe I could just drop it and do something like

// ...
const params = []
Object.keys(sample).map(key => params.push({'name': key, 'value': sample[key]}))

return {
  mimeType: 'application/x-www-form-urlencoded',
  params: params,
  text: Object.keys(sample).map(key => key + '=' + sample[key]).join('&'),
}
// ...

What do you think?

d-silva commented 3 years ago

Hello @ErikWittern

What do you think about the last changes?

Cheers

ErikWittern commented 3 years ago

@d-silva Sorry for coming back to you only now, I was on vacation.

I am generally ok with using a custom solution here instead of adding another dependency. However, given the specification, keys and values should both be escaped:

Control names and values are escaped. Space characters are replaced by +, and then reserved characters are escaped as described in [RFC1738], section 2.2: Non-alphanumeric characters are replaced by %HH, a percent sign and two hexadecimal digits representing the ASCII code of the character. Line breaks are represented as "CR LF" pairs (i.e., %0D%0A).

Maybe we can use encodeURIComponent for that purpose (even though I think it is not a perfect match, as that function does not replace whitespace with + as the spec wants, but rather with %20. Maybe you can find out if that is acceptable?).

Apart from that, there should be new tests covering this new behavior.

d-silva commented 3 years ago

@ErikWittern No worries, meanwhile I went on vacation too.

Hope you had a great vacation! 👍

About the %20 vs + subject, I didn't find a consensus on this matter and I even looked into Postman resulting code snippets and they sometimes use the %20 escaped space form (I want to believe most systems will support both forms 🤷‍♂️).

But like you mentioned, the specification explicitly says we should replace the %20 by + so I did it by using the encodeURIComponent and then replacing the resulting %20 by +

Let me know what you think. Cheers

d-silva commented 3 years ago

@ErikWittern Thank you so much for your help along the process 🍻