haraka / Haraka

A fast, highly extensible, and event driven SMTP server
https://haraka.github.io
MIT License
5.02k stars 662 forks source link

"TypeError: The header content contains invalid characters" when piping message_stream to request #1760

Closed shurli closed 7 years ago

shurli commented 7 years ago

Have this issue with transaction.message_stream.pipe(request.post( )....

2017-01-10T15:02:35.123Z [CRIT] [-] [core] TypeError: The header content contains invalid characters 2017-01-10T15:02:35.123Z [CRIT] [-] [core] at ClientRequest.OutgoingMessage.setHeader (_http_outgoing.js:358:11) 2017-01-10T15:02:35.123Z [CRIT] [-] [core] at new ClientRequest (_http_client.js:86:14) 2017-01-10T15:02:35.124Z [CRIT] [-] [core] at Object.exports.request (http.js:31:10) 2017-01-10T15:02:35.124Z [CRIT] [-] [core] at Object.exports.request (https.js:199:15) 2017-01-10T15:02:35.124Z [CRIT] [-] [core] at Request.start (/usr/lib/node_modules/request/index.js:603:30) 2017-01-10T15:02:35.124Z [CRIT] [-] [core] at Request.write (/usr/lib/node_modules/request/index.js:1226:28) 2017-01-10T15:02:35.125Z [CRIT] [-] [core] at MessageStream.ondata (stream.js:31:26) 2017-01-10T15:02:35.125Z [CRIT] [-] [core] at emitOne (events.js:96:13) 2017-01-10T15:02:35.125Z [CRIT] [-] [core] at MessageStream.emit (events.js:188:7) 2017-01-10T15:02:35.125Z [CRIT] [-] [core] at ChunkEmitter. (/usr/lib/node_modules/Haraka/messagestream.js:353:18)

on Haraka 2.8.12, request 2.79.0, node 6.9.4

same code has no error with Haraka 2.8.11, request 2.65.0 and node 4.2.6

I tried to go back to previous Haraka and request verison, same issue. I'll try to back to latest node 4.x.x, but have not tested this.

smfreegard commented 7 years ago

I don't think this is a Haraka problem. The problem is with the request library throwing an un-catchable error. I had this problem elsewhere (for a different reason) and I ended up hacking the request source code as a quick fix.

shurli commented 7 years ago

By studiying the changelogs of node it seems that this error comes with node 5.6.0 where these header checks are implemented, which throws the TypeError.

As already mentioned will test this with latest v4 tomorrow.

shurli commented 7 years ago

it was backported to node 4.3.0. -> so node 4.2.6 ist the last version without this issue. haraka says it needs 4.6.2, are there side effect running it at 4.2.6?

@smfreegard: I dont know how to catch this error in request code, and so make it work and prevent haraka vom crashing, I would know how to "fix" it in the node sources '_http_outgoing.js'

How to address the real problem that the messege_stream.pipe tries to set invalid_charaters at the headers? I thouth of it set s the body of request and not the headers?

shurli commented 7 years ago

I think I understand the problem of the issue!? As request can pipe to another request it tries to copy http headers

self.on('pipe', function (src) {
    if (self.ntick && self._started) {
      self.emit('error', new Error('You cannot pipe to this stream after the outbound request has started.'))
    }
    self.src = src
    if (isReadStream(src)) {
      if (!self.hasHeader('content-type')) {
        self.setHeader('content-type', mime.lookup(src.path))
      }
    } else {
      if (src.headers) {
        for (var i in src.headers) {
          if (!self.hasHeader(i)) {
            self.setHeader(i, src.headers[i])
          }
        }
      }
      if (self._json && !self.hasHeader('content-type')) {
        self.setHeader('content-type', 'application/json')
      }
      if (src.method && !self.explicitMethod) {
        self.method = src.method
      }
}

but using a transaction.message_stream is source stream headers is set with mailheaders. so it trys to copy the mailheaders into http header fields where they should not be, and were some characters are not allowed.

How can I remove or rename the message_stream.headers before piping?

smfreegard commented 7 years ago

As request can pipe to another request it tries to copy http headers

Correct - the issue is that you can't catch these errors from request, it doesn't propagate them.

How can I remove or rename the message_stream.headers before piping?

You can't.

You would have to check these in a plugin an reject any message which contained high-bit or invalid characters in the headers.

baudehlo commented 7 years ago

Well technically you could delete message_stream.headers before piping. And then maybe restore it afterwards.

On Thu, Jan 12, 2017 at 2:31 PM, Steve Freegard notifications@github.com wrote:

As request can pipe to another request it tries to copy http headers

Correct - the issue is that you can't catch these errors from request, it doesn't propagate them.

How can I remove or rename the message_stream.headers before piping?

You can't.

You would have to check these in a plugin an reject any message which contained high-bit or invalid characters in the headers.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/haraka/Haraka/issues/1760#issuecomment-272258741, or mute the thread https://github.com/notifications/unsubscribe-auth/AAobY1pDhIrwZdfOWSfkqBVpP0xV-6BBks5rRn93gaJpZM4LfmHX .

shurli commented 7 years ago

delete trans.message_stream.headers;

doesn't work.

2017-01-12T20:21:54.688Z [CRIT] [-] [core] TypeError: Cannot read property 'length' of undefined 2017-01-12T20:21:54.688Z [CRIT] [-] [core] at MessageStream._read (/usr/lib/node_modules/Haraka/messagestream.js:224:21)

used

trans.message_stream.headers = {};

instead.

It works juhu ;)

versions used: Haraka 2.8.12, request 2.79.0, node 6.9.4

baudehlo commented 7 years ago

You probably want to save off the old value and restore it:

var tmp_headers = trans.message_stream.headers; trans.message_stream.headers = {}; trans.message_stream.pipe(...); trans.message_stream.headers = tmp_headers;

On Thu, Jan 12, 2017 at 3:40 PM, shurli notifications@github.com wrote:

delete trans.message_stream.headers;

doesn't work.

2017-01-12T20:21:54.688Z [CRIT] [-] [core] TypeError: Cannot read property 'length' of undefined 2017-01-12T20:21:54.688Z [CRIT] [-] [core] at MessageStream._read (/usr/lib/node_modules/Haraka/messagestream.js:224:21)

used

trans.message_stream.headers = {};

instead.

It works juhu ;)

versions used: Haraka 2.8.12, request 2.79.0, node 6.9.4

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/haraka/Haraka/issues/1760#issuecomment-272277088, or mute the thread https://github.com/notifications/unsubscribe-auth/AAobY5NtvDe-0zEDlYz5L3umKjNIoTZtks5rRo-2gaJpZM4LfmHX .