-
Notifications
You must be signed in to change notification settings - Fork 459
InvalidAsn1Error: Expected 0x2: got 0x1 #142
Comments
Some of the code related to timeout events has been fixed up. |
I can confirm that this still happens, using ldapjs@0.7.0 from npmjs.org on node v0.10.24. I am now on OS X, whereas I was using Linux the last time I reported this issue. The symptom is still the same: half the time I get: Here is the stacktrace in the case of the latter error:
My code looks like this (minus the sanitized search string):
This is against an Oracle OID server, which is either broken or misconfigured (this is just the backup server, our primary server works fine). I am happy to provide a new Wireshark output of this conversation. Please reopen this issue, as I lack the permissions to do so. |
Yeah, the wireshark dump would be helpful. |
By email, pfmooney wrote:
That analysis is adequate, though I'm still not quite sure how to solve this. Regardless of the unanticipated response, I would expect ldapjs to handle it gracefully and not crash. I believe there are two independent fixes here, which are not mutually exclusive:
|
When the connectTimeout is reached, the client will fire that event if the calling code has registered for it. I believe it's an acceptable result given the circumstances. I'll look into what it will take to provide mechanisms to prevent protocol-related exceptions from bubbling up like in this case. |
At your leisure, could give 7c7c480 a try and see if it more gracefully handles the bogus data from that backup OID server? I don't have good testing resources available at the moment, but when I'm back in the office on Monday, I'll do more verification to ensure I haven't broken other things with the change. Thanks |
@raztus Have you had a chance to test that minor patch to parser error handling? This is one of the few remaining bugs I want to close out before a new release is tagged. |
I apologize, last week was busy. I'm ready to test this now, but when I attempt to checkout 7c7c480, I get |
No problem. The commit is over in my repository: https://github.com/pfmooney/node-ldapjs.git |
Gotcha. Scratch that. Now I've also learned that Github will show you commits from other forks as if they exist in the main repo. Now I know that too! |
Okay, I can confirm that that fix appears to solve the more serious issue, namely throwing an uncaught exception (condition (1)). However, the side-effect is that now condition (1) fires two "error" events, handled twice by the registered handler. Condition (2) (the ECONNRESET event) still fires just one "error" event. This is acceptable for my use case, but does seem funky. It seems to me that a more appropriate solution would be for the client to close the connection after connectTimeout ms. In this way it totally severs the link, and doesn't have to care what the server sends. As a consumer of ldapjs, this is what I expected to happen; that is, after connectTimeout ms, consider it a lost cause and move on, rather than still listening for further data. |
Yeah, the commit linking was a little weird. Regarding the error behavior, I have a couple comments:
|
Ahh, I understand what you're saying. I really meant to say "close the connection after timeout ms", but now I understand that even that is certainly not the right solution, since you wouldn't want to close the TCP connection just because one LDAP operation timed out. I think what I actually wanted was a way to force the client to unbind and close the connection, which looks like is available as I just discovered that if I put Below is the output from the two error events:
|
Thanks for your work on this, @pfmooney! It will be helpful. |
I see this les/ldapjs/lib/client/client.js:1282:14\n at Array.forEach (native)\n at Client._onClose (/home/amills/AdminUI/node_modules/ldapjs/lib/client/client.js:1272:19)\n at TLSSocket.g (events.js:260:16)\n at emitOne (events.js:77:13)\n at TLSSocket.emit (events.js:169:7)\n at TCP._onclose (net.js:469:12)","time":"2016-03-24T03:01:39.750Z","src":{"file":"/home/amills/AdminUI/app.js","line":265},"v":0}
uncaughtException--->Error: read ECONNRESET
at exports._errnoException (util.js:856:11)
at TLSWrap.onread (net.js:544:26)
/home/amills/AdminUI/bin/www.js:17
throw err; //this should crash the server
^
Error: read ECONNRESET
at exports._errnoException (util.js:856:11)
at TLSWrap.onread (net.js:544:26) an ECONNRESET throws an uncatchable exception or should I be able to catch it somehow? |
I'm experiencing this error after several minutes of a bad connection. The LDAP server I'm connecting to successfully establishes a TCP connection (
connect
event is fired andonConnect
executes, removing theconnectTimeout
timer), but then doesn't respond for about 7 minutes, at which point I experience one of two conditions:InvalidAsn1Error: Expected 0x2: got 0x1
or[TRACE] { '0': { err: { [Error: read ECONNRESET] code: 'ECONNRESET', errno: 'ECONNRESET', syscall: 'read' } }, '1': 'error event: %s', '2': 'Error\n at Socket.onError ...
[TRACE] { '0': 'close event had_err=%s', '1': 'yes' }
Condition (1), which causes my application to crash, is more common. Here is a simplified excerpt of my code:
I can send you the wireshark output of this conversation, if you'd like.
One hack that solves this issue for me, but which is obviously incorrect, is to add
conn.destroy()
infunction onRequestTimeout()
(about line 868).The text was updated successfully, but these errors were encountered: