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

Issue handling multiple types in one header #16

Closed
tunnckoCore opened this issue Jul 2, 2015 · 7 comments
Closed

Issue handling multiple types in one header #16

tunnckoCore opened this issue Jul 2, 2015 · 7 comments
Assignees
Labels

Comments

@tunnckoCore
Copy link

stated at helapkg/hela#22

Problem is that some API service - Alipay, sending request content-type header like that

application/x-www-form-urlencoded; text/html; charset=utf-8

I dont know why they send it like that, but there's no logic for me. We've tried in different ways, but didn't work.

is(req, ['x-www-form-urlencoded', 'html'])
is(req, ['application/x-www-form-urlencoded', 'text/html'])

It's not koa-better-body implementation issue, cuz it support extendTypes option that pass things to koa this.is/this.request.is

@tunnckoCore
Copy link
Author

I think it would not be correct to support (if cant currently) thing like that and maybe we should pass the problem to Alipay tracker to fix their header?

@dougwilson dougwilson self-assigned this Jul 2, 2015
@dougwilson
Copy link
Contributor

Right, this module explicitly rejects that header value on purpose. The reason is that we detect if the type of a request is something; if the value in the content-type header is not a valid type, we're no longer comparing types, but rather random strings. In order to do intelligent type comparisons, like ignoring the parameters, we have to at least know it's actually a content-type header value.

There are really only two good courses of action here:

  1. Get them to fix their header.
  2. Re-write the header value before calling this module, either by altering the content-type header or calling is.is with the altered type (probably after calling is.hasBody).

@dougwilson
Copy link
Contributor

You can also find some previous discussion on this exact Alipay issue in #12

@dougwilson
Copy link
Contributor

It's also amusing to search Google for the string "application/x-www-form-urlencoded; text/html; charset=utf-8", as you see people using all kind of different language like Ruby, Java, C#, and more also having issues with Alipay's dumb header.

@dougwilson
Copy link
Contributor

And for reference, you can find the exact format the Content-Type's value needs to be in at http://tools.ietf.org/html/rfc7231#section-3.1.1.5 . The issue with that header is that the second semicolon is an illegal character and no longer matches the RFC's ABNF expression for the Content-Type header.

@tunnckoCore
Copy link
Author

Hm, okey. Just didn't have time to look at google.
So Alipay issue and we won't do anything, and with reason.

So @daiwhea, that's the way currently.

app.use(function * (next) {
  if (this.request.header['content-type'] === 'application/x-www-form-urlencoded; text/html; charset=utf-8') {
    this.request.header['content-type'] = 'application/x-www-form-urlencoded'
    // or 
    // this.request.header['content-type'] = 'application/x-www-form-urlencoded; charset=utf-8'
  }
  yield next
})

I'll think to do something on koa-better-body.

@dougwilson
Copy link
Contributor

One option with koa-better-body is you could perhaps even take a function as the type to match, which is what I allow with the body-parser module (https://github.com/expressjs/body-parser#type).

If a function, the type option is called as fn(req) and the request is parsed if it returns a truthy value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants