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

Unsupported BodyInit type #105

Open
jasonhjohnson opened this issue Mar 16, 2016 · 15 comments
Open

Unsupported BodyInit type #105

jasonhjohnson opened this issue Mar 16, 2016 · 15 comments

Comments

@jasonhjohnson
Copy link

So, in IE 9 when I tried using this plugin I got the following error after authenticating in the popup:
Blob is undefined

So, I figured a Blob polyfill might be needed and threw this in my pages header: https://rawgit.com/eligrey/Blob.js/master/Blob.js

That seemed to fix the Blob error. But then, got a different error:
Unsupported BodyInit type

I thought it might have something to do with the fetch polyfil so I tried using the fetch-ie8 polyfill instead but it didn't fix it. Any help is appreciated.

Thanks,
Jason

@jasonhjohnson
Copy link
Author

I think it may be related to this issue: facebook/react-native#2538.

Perhaps adjust the POST in your login() method to something like:

return this.http.fetch(loginUrl, {
      method: 'post',
      headers: typeof(content)==='string' ? {'Content-Type': 'application/x-www-form-urlencoded', 'Accept': 'application/json'} : {},
      body: typeof(content)==='string' ? JSON.stringify(content) : JSON.stringify(content)
    })     
  }

@jasonhjohnson
Copy link
Author

Actually, it's probably needed in the exchangeForToken method of oauth2.js

@jasonhjohnson
Copy link
Author

I got this to work in IE 9 (still works in other browsers) by changing the POST in exchangeForToken to the following:

return this.http.fetch(exchangeForTokenUrl, {
      method: 'post',
      headers: { 'Content-Type': 'application/json; charset=utf-8' },
      body: JSON.stringify(data),
      credentials: credentials
    })

Could you incorporate this change?

Thanks,
J

@mbroadst
Copy link
Contributor

@jasonhjohnson put together a PR we'll get it in

@jasonhjohnson
Copy link
Author

@mbroadst Sure thing. Also, you guys may not care about IE 9 (that sounded snotty but I didn't mean it that way), but in addition to the change above I had to use two polyfills:

<script src="//cdn.rawgit.com/eligrey/Blob.js/master/Blob.js"></script>
<script src="//cdn.rawgit.com/davidchambers/Base64.js/master/base64.min.js"></script>

@mbroadst
Copy link
Contributor

@jasonhjohnson yeah I think that information is probably better placed directly on the README, I don't think it makes sense to include the polyfill since most browsers already support those (plus this is more related to fetch rather than aurelia-auth)

@jasonhjohnson
Copy link
Author

@mbroadst Agreed. #107

@ghiscoding
Copy link
Contributor

ghiscoding commented Nov 18, 2016

I actually got this problem with the Twitter and OAuth1.js and Koa (server side). It seems that Koa doesn't like the POST with missing Content-Type, I get the following error

  InternalServerError: Missing content-type
      at Object.throw (C:\OwnSyncDev\aurelia-auth-sample\server\node_modules\koa\lib\context.js:91:23)
      at Object.exports.authenticate (C:\OwnSyncDev\aurelia-auth-sample\server\auths\twitter.js:110:22)
      at exports.authenticate.throw (<anonymous>)

I traced down the problem to be in the oauth1.js file.

OAuth1.prototype.exchangeForToken = function exchangeForToken(oauthData, userData, current) {
      var data = (0, _authUtilities.extend)({}, userData, oauthData);
      var exchangeForTokenUrl = this.config.baseUrl ? (0, _authUtilities.joinUrl)(this.config.baseUrl, current.url) : current.url;
      var credentials = this.config.withCredentials ? 'include' : 'same-origin';

      return this.http.fetch(exchangeForTokenUrl, {
        method: 'post',
        body: (0, _aureliaFetchClient.json)(data),
        credentials: credentials
      }).then(_authUtilities.status);

Adding a Content-Type on that http.fetch fixes the problem. However like @paulvanbladel suggested in the PR #107, the Content-Type could also be added in the auth-fetch-config.

So here's the final fix which works for me

this.httpClient.configure(function (httpConfig) {
        httpConfig.withDefaults({
          headers: {
            'Accept': 'application/json',
            'Content-Type': 'application/json' // <<-- this line -->>
          }
        }).withInterceptor(_this.auth.tokenInterceptor);
      });

I can make a PR if really required

paulvanbladel added a commit that referenced this issue Dec 16, 2016
Add missing header in Fetch-Config issue #105 #107
@alexsaare
Copy link

alexsaare commented Jul 4, 2017

This fix breaks the default behaviour when posting multipart data. The boundary information of the Content Type header should be dynamically added by the browser. Forcing the content type to appication/json means that you can not remove it and allow aurelia-fetch to do it's own thing

import {HttpClient, json} from 'aurelia-fetch-client';

@inject(HttpClient)
export class MyClass {

constructor(private client : HttpClient){} 

public createDocument(fileKey :string, files : FileList) : Promise<MyResponse> {
    var form = new FormData();

    for (let i = 0; i < files.length; i++) {
       form.append(fileKey, files[i]);
     }

    return this.client
       .fetch('someurl', { method: 'post', body: form })
       .then(response => {
             ...process response
       });
}

Setting it to undefined as part of the fetch (as suggested here) actually outputs the string "undefined" in the header - same for null. I.E. the below doesn't work.

    .fetch('someurl', { method: 'post', body: form, headers: {'ContentType': undefined } })

I can create a new HttpClient but then i can't add the interceptor as the Authentication module is not exposed.

Let me know if i'm missing something!

@ielcoro
Copy link
Collaborator

ielcoro commented Nov 13, 2017

Please revert the merged Pull Request #168 and set the headers explicitly on request instead of messing with the defaults.

Versión 3.0.5 totally broke various ongoing applications, because as @alexsaare said, the defaults prevent multipart header from being calculated automatically when using FormData.

For the time being we don't use FetchConfig for configuring the library and instead we rely on manual configuration:

function configureHttpClient(aurelia: Aurelia): void {
    let httpClient: HttpClient = aurelia.container.get(HttpClient);
    let auth = aurelia.container.get(AuthService);

     // Do not use FetchConfig as #105 broke all multipart requests, instead configure it manually
    // let httpClientAuth: FetchConfig = aurelia.container.get(FetchConfig);

    // httpClientAuth.configure();
    
    //manually configure httpclient auth interceptor
    httpClient.configure(config => {
        config.withInterceptor(auth.tokenInterceptor)

@ielcoro
Copy link
Collaborator

ielcoro commented Nov 13, 2017

Just uploaded a pull request that reverts this behavior and instead uses a explicit per-request headers approach.

@alexsaare
Copy link

Just FYI -

To work around this issue, I extended HttpClient like this

import {HttpClient} from 'aurelia-fetch-client';
import {inject} from 'aurelia-framework';
import {AuthService} from 'aurelia-auth';

@inject(AuthService)
export class NonJsonHttpClient extends HttpClient {
  constructor(auth) {
    super();
    this.configure(config => {
      config
        .withInterceptor(auth.tokenInterceptor);
    });
  }
}

So for the classes that need MultiPart headers I inject NonJsonHttpClient rather than HttpClient.

This gives me a different instance which works as expected.

@paulvanbladel
Copy link
Owner

@ielcoro are you interested to join as admin and handle the PR?

@ielcoro
Copy link
Collaborator

ielcoro commented Nov 13, 2017

@paulvanbladel it would be a pleasure. However I have to be honest, I do not have much time to spare on side projects, at least not as much as I would love, so I can do some code reviews and issue filtering, pr mergin etc, but I don't have the time to take the ownership of the repo, if that is ok with you, yes I can start handling this PR.

@paulvanbladel
Copy link
Owner

Thanks.

ielcoro added a commit that referenced this issue Nov 15, 2017
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

No branches or pull requests

6 participants