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

Support form url encoding #666

Closed
wants to merge 4 commits into from
Closed

Conversation

iamdanfox
Copy link
Contributor

@iamdanfox iamdanfox commented Apr 22, 2020

Before this PR

The OAuth2 spec allows people to send data in the body as long as it is encoded sent with content type application/x-www-form-urlencoded.

https://tools.ietf.org/html/rfc6749#section-3.2

     POST /token HTTP/1.1
     Host: server.example.com
     Content-Type: application/x-www-form-urlencoded

     grant_type=refresh_token&refresh_token=tGzv3JOkF0XG5Qx2TlKWIA
     &client_id=s6BhdRkqt3&client_secret=7Fjfp0ZBr1KtDRbnfVdmIw

We have no intention of supporting this encoding as a way of defining conjure APIs, but I'd like people to use dialogue to make calls to MP, which requires this. cc @pkoenig10

After this PR

==COMMIT_MSG==
The BodySerDe#formUrlEncodingSerializer can be used to send application/x-www-form-urlencoded data (e.g. for OAuth2 endpoints).
==COMMIT_MSG==

Possible downsides?

  • it muddies up the nice BodySerDe interface a bit.

@changelog-app
Copy link

changelog-app bot commented Apr 22, 2020

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

The BodySerDe#formUrlEncodingSerializer can be used to send application/x-www-form-urlencoded data (e.g. for OAuth2 endpoints).

Check the box to generate changelog(s)

  • Generate changelog entry

@policy-bot policy-bot bot requested a review from CRogers April 22, 2020 20:08
@iamdanfox iamdanfox requested review from carterkozak and ferozco and removed request for CRogers April 22, 2020 20:09
@carterkozak
Copy link
Contributor

carterkozak commented Apr 22, 2020

I'm not sure I understand why this should live in dialogue instead of the projects that intend to use it or a separate utility library.

I'm not convinced that map<str,str> is the ideal data type in this case. If there are numeric values do we expect consumers to effectively serialize those into strings prior to passing data through this serializer? What about collections of values like query parameters?

@iamdanfox
Copy link
Contributor Author

Closing out because I think the URLEncoder.encode is pretty non-scary to use internally.

@iamdanfox iamdanfox closed this Apr 23, 2020
@iamdanfox iamdanfox deleted the dfox/form-url-encoding branch April 27, 2020 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants