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

Content-Encoding issue #100

Closed
cha55son opened this issue May 11, 2015 · 5 comments
Closed

Content-Encoding issue #100

cha55son opened this issue May 11, 2015 · 5 comments
Assignees
Labels

Comments

@cha55son
Copy link

I've got an application that is sending webhooks to hubot. I receive the error pasted below. Unfortunately the app is sending webhooks with a header of Content-Encoding: UTF-8 and there is nothing I can do on that end. I can change hubot however I need to though. Is there a way to get express/body-parser to "ignore" the Content-Encoding header?

This used to work with an older version of hubot. I just recently upgraded and i'm now hitting this error. Any help or direction would be appreciated.

Error: unsupported content encoding "utf-8"
  at contentstream (/home/cchoate/drbot/node_modules/hubot/node_modules/express/node_modules/connect/node_modules/body-parser/lib/read.js:146:13)
  at read (/home/cchoate/drbot/node_modules/hubot/node_modules/express/node_modules/connect/node_modules/body-parser/lib/read.js:42:14)
  at Object.jsonParser [as handle] (/home/cchoate/drbot/node_modules/hubot/node_modules/express/node_modules/connect/node_modules/body-parser/lib/types/json.js:96:5)
  at next (/home/cchoate/drbot/node_modules/hubot/node_modules/express/node_modules/connect/lib/proto.js:169:15)
  at Object.query [as handle] (/home/cchoate/drbot/node_modules/hubot/node_modules/express/node_modules/connect/lib/middleware/query.js:43:5)
  at next (/home/cchoate/drbot/node_modules/hubot/node_modules/express/node_modules/connect/lib/proto.js:169:15)
  at Object.handle (/home/cchoate/drbot/node_modules/hubot/src/robot.coffee:288:7, <js>:243:16)
  at next (/home/cchoate/drbot/node_modules/hubot/node_modules/express/node_modules/connect/lib/proto.js:169:15)
  at Object.expressInit [as handle] (/home/cchoate/drbot/node_modules/hubot/node_modules/express/lib/middleware.js:30:5)
  at next (/home/cchoate/drbot/node_modules/hubot/node_modules/express/node_modules/connect/lib/proto.js:169:15)
  at Object.query [as handle] (/home/cchoate/drbot/node_modules/hubot/node_modules/express/node_modules/connect/lib/middleware/query.js:43:5)
  at next (/home/cchoate/drbot/node_modules/hubot/node_modules/express/node_modules/connect/lib/proto.js:169:15)
  at Function.app.handle (/home/cchoate/drbot/node_modules/hubot/node_modules/express/node_modules/connect/lib/proto.js:177:3)
  at Server.app (/home/cchoate/drbot/node_modules/hubot/node_modules/express/node_modules/connect/lib/connect.js:67:37)
  at Server.EventEmitter.emit (events.js:98:17)
  at HTTPParser.parser.onIncoming (http.js:2108:12)
  at HTTPParser.parserOnHeadersComplete [as onHeadersComplete] (http.js:121:23)
  at Socket.socket.ondata (http.js:1966:22)
  at TCP.onread (net.js:525:27)
@dougwilson
Copy link
Contributor

You can always add the following prior to invoking the body parser middleware:

app.use(function (req, res, next) {
  delete req.headers['content-encoding']
  next()
})

and of course you can always wrap that delete in an if if you want to be more selective. RFC 7231 sec 3.1.2.2 has the following to say:

An origin server MAY respond with a status code of 415 (Unsupported
Media Type) if a representation in the request message has a content
coding that is not acceptable.

So we could potentially make this error optional.

@dougwilson dougwilson self-assigned this May 11, 2015
@cha55son
Copy link
Author

Thanks for the quick response. I'll be sure to give this a try tomorrow. Hopefully there is a way to insert this middleware before the body parser runs.

@dougwilson
Copy link
Contributor

Hm, I see. Looking at https://github.com/github/hubot/blob/5ec8f375f3ac97e9df6e45c6b459d42025b74694/src/robot.coffee it seems pretty self-contained, so if this is possible, it's not going to be simple. Even then, if I add a configuration here, you'll still have to get hubot to set the config value on this module internally.

@cha55son
Copy link
Author

After reviewing hubot's code I believe you're right. They really need to add a way to insert middleware before they add their own, for cases like this. I might look at submitting a PR for that functionality.

I don't believe there are any issues with the way expressjs is handling that Content-Encoding header. Per the spec, Content-Encoding: UTF-8 doesn't make any sense anyway.

Thank you for your cooperation and guidance. I believe we can close this ticket and i'll move this conversation over to the hubot repo.

@dougwilson
Copy link
Contributor

So, I meant to do this earlier, but kept forgetting: I added an issue #108 that would, in situations like this, allow you to add a custom content encoding hook. For instance, you would eventually be able to theoretically add a hook for UTF-8 encoding and just return the original stream (so, make it an alias for identity).

@expressjs expressjs locked and limited conversation to collaborators Jun 14, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants