haraka / Haraka

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

client.socket.remoteAddress undefined #1372

Closed msimerson closed 8 years ago

msimerson commented 8 years ago

related to #1361

Node.js version

Tested on v4 and v5

Haraka version

Starting up Haraka version 2.8.0-alpha.5

Yes, alpha.5 (and later) includes the patch in PR #1362

Expected behavior

not to crash.

Observed behavior

[INFO] [-] [core] [smtp_client_pool] [25:127.0.0.6:300] dispense() clients=1 available=0
[CRIT] [-] [core] Error: ipaddr: the address has neither IPv6 nor IPv4 format
[CRIT] [-] [core]     at Object.ipaddr.parse (/usr/local/lib/node_modules/Haraka/node_modules/ipaddr.js/lib/ipaddr.js:438:13)
[CRIT] [-] [core]     at Object.ipaddr.process (/usr/local/lib/node_modules/Haraka/node_modules/ipaddr.js/lib/ipaddr.js:459:17)
[CRIT] [-] [core]     at pluggableStream.<anonymous> (/usr/local/lib/node_modules/Haraka/smtp_client.js:131:35)
[CRIT] [-] [core]     at emitTwo (events.js:87:13)
[CRIT] [-] [core]     at pluggableStream.emit (events.js:172:7)
[CRIT] [-] [core]     at Socket.<anonymous> (/usr/local/lib/node_modules/Haraka/tls_socket.js:51:14)
[CRIT] [-] [core]     at emitNone (events.js:72:20)
[CRIT] [-] [core]     at Socket.emit (events.js:166:7)
[CRIT] [-] [core]     at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1048:10)

Steps to reproduce

diff --git a/smtp_client.js b/smtp_client.js
index 0117758..a912cd5 100644
--- a/smtp_client.js
+++ b/smtp_client.js
@@ -128,7 +128,13 @@ function SMTPClient (port, host, connect_timeout, idle_timeout) {
     this.socket.on('connect', function () {
         // Remove connection timeout and set idle timeout
         client.socket.setTimeout(((idle_timeout) ? idle_timeout : 300) * 1000);
-        client.remote_ip = ipaddr.process(client.socket.remoteAddress).toString();
+        if (client.socket.remoteAddress) {
+            // "Value may be undefined if the socket is destroyed"
+            client.remote_ip = ipaddr.process(client.socket.remoteAddress).toString();
+        }
+        else {
+            logger.logerror('client.socket.remoteAddress undefined!');
+        }
     });

     var closed = function (msg) {
msimerson commented 8 years ago

See net api docs for remoteAddress.

celesteking commented 8 years ago

The funny thing is that the client doesn't disconnect yet at the time of error. Something's fishy with nodejs.

pluggableStream {
  domain: null,
  _events: 
   { data: [Function],
     end: [ [Function], [Function] ],
     line: [Function],
     connect: [Function],
     error: [Function],
     timeout: [Function],
     close: [Function] },
  _eventsCount: 7,
  _maxListeners: undefined,
  writable: true,
  readable: true,
  _timeout: 300000,
  _keepalive: true,
  _writeState: true,
  _pending: [],
  _pendingCallbacks: [],
  targetsocket: 
   Socket {
     _connecting: false,
     _hadError: false,
     _handle: 
      TCP {
        _externalStream: {},
        fd: 23,
        reading: false,
        owner: [Circular],
        onread: [Function: onread],
        onconnection: null,
        writeQueueSize: 0 },
     _parent: null,
     _host: null,
     _readableState: 
      ReadableState {
        objectMode: false,
        highWaterMark: 16384,
        buffer: [],
        length: 0,
        pipes: null,
        pipesCount: 0,
        flowing: true,
        ended: false,
        endEmitted: false,
        reading: true,
        sync: false,
        needReadable: true,
        emittedReadable: false,
        readableListening: false,
        defaultEncoding: 'utf8',
        ranOut: false,
        awaitDrain: 0,
        readingMore: false,
        decoder: null,
        encoding: null,
        resumeScheduled: false },
     readable: true,
     domain: null,
     _events: 
      { end: [Object],
        finish: [Function: onSocketFinish],
        _socketEnd: [Function: onSocketEnd],
        data: [Function],
        connect: [Object],
        secureConnection: [Function],
        secure: [Function],
        close: [Function],
        drain: [Function],
        error: [Function],
        timeout: [Function] },
     _eventsCount: 11,
     _maxListeners: undefined,
     _writableState: 
      WritableState {
        objectMode: false,
        highWaterMark: 16384,
        needDrain: false,
        ending: false,
        ended: false,
        finished: false,
        decodeStrings: false,
        defaultEncoding: 'utf8',
        length: 0,
        writing: false,
        corked: 0,
        sync: true,
        bufferProcessing: false,
        onwrite: [Function],
        writecb: null,
        writelen: 0,
        bufferedRequest: null,
        lastBufferedRequest: null,
        pendingcb: 0,
        prefinished: false,
        errorEmitted: false },
     writable: true,
     allowHalfOpen: false,
     destroyed: false,
     bytesRead: 0,
     _bytesDispatched: 0,
     _sockname: null,
     _pendingData: null,
     _pendingEncoding: '',
     _idleTimeout: 300000,
     _idleNext: 
      Socket {
        _connecting: false,
        _hadError: false,
        _handle: [Object],
        _parent: null,
        _host: null,
        _readableState: [Object],
        readable: true,
        domain: null,
        _events: [Object],
        _eventsCount: 11,
        _maxListeners: undefined,
        _writableState: [Object],
        writable: true,
        allowHalfOpen: false,
        destroyed: false,
        bytesRead: 384,
        _bytesDispatched: 402,
        _sockname: null,
        _pendingData: null,
        _pendingEncoding: '',
        server: [Object],
        _peername: [Object],
        _idleTimeout: 600000,
        _idleNext: [Object],
        _idlePrev: [Circular],
        _idleStart: 3853,
        read: [Function],
        _consuming: true },
     _idlePrev: { _idleNext: [Circular], _idlePrev: [Object] },
     _idleStart: 3924,
     read: [Function],
     _consuming: true },
  upgrade: [Function],
  process_data: [Function],
  process_end: [Function] }
longlostnick commented 8 years ago

Running into the same problem with node v5.7.1 and Haraka 2.8.0-alpha.7

The temporary workaround seems to take care of it. Weird.

Ranger-X commented 8 years ago

Same trouble. Workaround is working. Thanks!