-
Notifications
You must be signed in to change notification settings - Fork 74
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
Added content type header and stringified JSON in body #107
Added content type header and stringified JSON in body #107
Conversation
The POST without the content type and stingfied body didn't work in IE 9
@@ -113,7 +113,8 @@ export class OAuth2 { | |||
|
|||
return this.http.fetch(exchangeForTokenUrl, { | |||
method: 'post', | |||
body: json(data), | |||
headers: { 'Content-Type': 'application/json; charset=utf-8' }, | |||
body: JSON.stringify(data), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just out of curiosity, does body: json(data)
still work? As in, is it only required that we add the headers here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I got the original error I mentioned when using json(data).
Sent from my iPhone
On Mar 17, 2016, at 9:09 AM, Matt Broadstone [email protected] wrote:
In src/oAuth2.js:
@@ -113,7 +113,8 @@ export class OAuth2 {
return this.http.fetch(exchangeForTokenUrl, { method: 'post',
body: json(data),
headers: { 'Content-Type': 'application/json; charset=utf-8' },
just out of curiosity, does body: json(data) still work? As in, is it only required that we add the headers here?body: JSON.stringify(data),
—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we not add the Content-Type header directly in the fetch config?
By doing so, it's added for all fetch request, but I don't see any harm in that.
Quite confused about changing from json(data)--> JSON.stringify for the body. Are we sure this is ok for the other browsers?
@mbroadst , enjoy the holiday!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jasonhjohnson is charset=utf-8 necessary for IE9 as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
give a read through Aurelia-fetch-client. From my reading the "json" helper should make a Blob, and then the client should add the appropriate headers. I think this is a but with Aurelia-fetch-client
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paulvanbladel The charset=utf-8 portion was necessary for IE 9.
@jasonhjohnson I'm going to leave this one for @paulvanbladel to review, as I'm headed out on vacation for a few days. At first glance it looks like you'd need to include this fix for oauth1 as well (they use the same code there it looks like), but furthermore it might actually be a bug in the aurelia-fetch-client. I'm not quite the fix belongs here, considering the code used to post this data is their suggested route for posting json data |
The POST without the content type and stingfied body didn't work in IE 9