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

OAuth2: use correct Content-Type as specified in RFC #2343

Merged
merged 4 commits into from
Jan 9, 2023

Conversation

vitalyster
Copy link
Contributor

@vitalyster vitalyster commented Jan 3, 2023

@@ -51,7 +51,7 @@ async function _fetch (url, fetchOptions, options) {
async function _putOrPostOrPatch (method, url, body, headers, options) {
const fetchOptions = makeFetchOptions(method, headers, options)
if (body) {
if (body instanceof FormData) {
if (body instanceof FormData || body instanceof URLSearchParams) {
fetchOptions.body = body
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that, by default, since the Content-Type is not set here, the browser (Chromium at least) sets

  • multipart/form-data; boundary=----WebKitFormBoundaryCBpNkZBPd2t28viK for FormData
  • application/x-www-form-urlencoded; charset=utf-8 for URLSearchParams

This is really surprising behavior to me. To me it would make more sense to explicitly specify the two content-types in the two different cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems to be documented in https://fetch.spec.whatwg.org/#bodyinit-unions

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fascinating, thanks!

@nolanlawson
Copy link
Owner

Thanks for opening this PR! Could you explain what problem this solves? E.g. is there a fediverse server that is broken with Pinafore's current approach?

@vitalyster
Copy link
Contributor Author

I'm implementing Mastodon API on my own server and that is the issue I spend hours to find why Pinafore doesn't sign in. It's better to make it use standard behavior to help new developers in the future.

@nolanlawson nolanlawson merged commit c426b7f into nolanlawson:master Jan 9, 2023
@vitalyster vitalyster deleted the oauth branch January 9, 2023 06:31
alice-werefox pushed a commit to alice-werefox/sema-werefox-cafe that referenced this pull request Apr 3, 2023
…son#2343)

Co-authored-by: Nolan Lawson <nolan@nolanlawson.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants