search

benjaminws / stomp-js

Implementation of the STOMP protocol in node.js
BSD 3-Clause "New" or "Revised" License
87 stars 47 forks source link

Handle frames with just NULL delim #13

Closed rauls closed 12 years ago

rauls commented 13 years ago

I made my tiny changes to handle the case of servers that send messages properly without a LF after the NULL at the of a frame.

benjaminws commented 13 years ago

I feel like I've been through this before ;).

Let me stew on it, drag up old commits/tweets/emails with the STOMP guys (whom I talked to about this because it wasn't officially in the spec).

rauls commented 13 years ago

I think in the specs, the docs said, its "OPTIONAL" to send LF after a NULL.

My problem is im dealing with another stomp based server that doesnt like these LFs between packets.

But at the very very least, the frame decoder SHOULD handle the case of a frame ending in JUST a NULL, and NO LF. That is a 100% valid frame.

The Perl version I have used doesn't send these LFs.

Personally, i think a server should be smart and deal with all issues/version of protocols, rather than the client try to handle all variations of servers. But the client has to decode the case of NO LFs too.

Raul

On Fri, Sep 30, 2011 at 12:50 AM, Benjamin W. Smith < reply@reply.github.com>wrote:

I feel like I've been through this before ;).

Let me stew on it, drag up old commits/tweets/emails with the STOMP guys (whom I talked to about this because it wasn't officially in the spec).

Reply to this email directly or view it on GitHub: https://github.com/benjaminws/stomp-js/pull/13#issuecomment-2238499

-Raul Sobon

raul.sobon@ac3dec.com or raul.sobon@gmail.com phone +61 404 998 990/0411 264 148, PennyTel-8889217619

benjaminws commented 13 years ago

Fair enough. Ill review and likely merge it in tomorrow and push it to npm provided it doesn't break anything else :).

rauls commented 13 years ago

I know the perl one doesnt do it, and I vagely remember that had same issue in PHP version, which might have removed the last \N too.

If there is a fundemental reason for it, id like to know. firewall/switch issues perhaps?

Raul

On Fri, Sep 30, 2011 at 2:09 PM, Benjamin W. Smith < reply@reply.github.com>wrote:

Fair enough. Ill review and likely merge it in tomorrow and push it to npm provided it doesn't break anything else :).

Reply to this email directly or view it on GitHub: https://github.com/benjaminws/stomp-js/pull/13#issuecomment-2246049

-Raul Sobon

raul.sobon@ac3dec.com or raul.sobon@gmail.com phone +61 404 998 990/0411 264 148, PennyTel-8889217619

rauls commented 13 years ago

Now I am only having one problem with this here, it looks like the last NULL sent via SSL only to a server (our own coded MQ server) from USA cloudfoundry.com app to australia , through a firewall, port mapped to a local server here.

But if run locally (ie on our work network, no public internet), it works.

I am not sure if its our server which uses openssl 1.0.0 , or if its firewalls , or the 30 routers between here to usa (doubt it , its SSL, unless CIA is checking and recrypting)

NODEJS on CF.com is still 0.4.5, and im running 0.4.12 locally here.

All debugs look sound, saying sent N bytes, same here as over in usa.

Sending NULL,\N means we only see NULL, and not \N from usa. But do see \N locally. Same as sending two NULLs, we only see one.

If you have a cloudfoundry.com account, try it from there via SSL talking to your home ActiveMQ or something else.

Any way, going to do more analysis of this, its really really weird!!!

On Fri, Sep 30, 2011 at 2:09 PM, Benjamin W. Smith < reply@reply.github.com>wrote:

Fair enough. Ill review and likely merge it in tomorrow and push it to npm provided it doesn't break anything else :).

Reply to this email directly or view it on GitHub: https://github.com/benjaminws/stomp-js/pull/13#issuecomment-2246049

-Raul Sobon

raul.sobon@ac3dec.com or raul.sobon@gmail.com phone +61 404 998 990/0411 264 148, PennyTel-8889217619

benjaminws commented 13 years ago

Oh that stinks! Let me know what you find out about that.

rauls commented 13 years ago

One question, when an SSL server 'dies' or is torn down, or loses comms, I get a failed to write to SSL stream or something to that effect.

How is this trapable? So we can retry a reconnect in 30 seconds.

Cheers

On Fri, Sep 30, 2011 at 11:39 PM, Benjamin W. Smith < reply@reply.github.com>wrote:

Oh that stinks! Let me know what you find out about that.

Reply to this email directly or view it on GitHub: https://github.com/benjaminws/stomp-js/pull/13#issuecomment-2249462

-Raul Sobon

raul.sobon@ac3dec.com or raul.sobon@gmail.com phone +61 404 998 990/0411 264 148, PennyTel-8889217619

benjaminws commented 13 years ago

I'm sure it can be trapped. I will see if I can trap it and emit an error to catch.

rauls commented 13 years ago

Another feature to add, is a flag or setting so you can auto defined content-length to all outgoing messages.

Cheers

On Tue, Oct 4, 2011 at 1:00 AM, Benjamin W. Smith < reply@reply.github.com>wrote:

I'm sure it can be trapped. I will see if I can trap it and emit an error to catch.

Reply to this email directly or view it on GitHub: https://github.com/benjaminws/stomp-js/pull/13#issuecomment-2272363

-Raul Sobon

raul.sobon@ac3dec.com or raul.sobon@gmail.com phone +61 404 998 990/0411 264 148, PennyTel-8889217619

benjaminws commented 12 years ago

I'm going to go ahead and merge this in. I've tested things, looks good.