stomp-js / stomp-websocket

Stomp client for Web browsers and node.js apps
https://stomp-js.github.io/stomp-websocket/
Apache License 2.0
141 stars 36 forks source link

option to omit 'content-length' header #24

Closed david-caruso closed 6 years ago

david-caruso commented 6 years ago

Currently we are using ActiveMQ as our back-end broker in our solution. Specifically, we are transmitting messages using the ActiveMQ MapMessage format. When sending messages in this format, including a 'content-length' header causes ActiveMQ to interpret the message differently as a ByteMessage instead of a MapMessage. I would like to request that an option be added to omit this header value from any frames.

Currently, in the Frame.prototype.toString() method, there is this code: skipContentLength = (this.headers['content-length'] === false) ? true : false;

And then later on, code that checks for skipContentLength if (this.body && !skipContentLength) { lines.push("content-length:" + (Frame.sizeOfUTF8(this.body))); }

Presumably, this code was put there to allow the this header to be omitted if the condition evaluates to true. However, because of the strict equality comparison, the only way to get this condition to be true is to set the 'content-length' header value to 'false', which is pretty clunky since it is meant to be an integer value.

In addition, even if you get this condition to evaluate to true, the following line will actually delete the header value from the cached header object: if (skipContentLength) { delete this.headers['content-length']; }

So, the header will be removed during the first "CONNECT" or "SEND" frame correctly, but on any subsequent frames, the skipContentLength = (this.headers['content-length'] === false) ? true : false; condition above will now evaluate to false, because this.headers['content-length'] is now undefined (doesn't exist), and the header will again be added to the frame.

To fix this, I'd like to propose one of the following: 1) Add a boolean option in the Frame function constructor called 'skipContentLength', that just prevents the content-length header from being sent regardless of whether that header exists or what it's value is, and also do not "delete" it from the cached header object?

// do not do this anymore. this comes from a boolean option value passed in the Frame constructor and stored //skipContentLength = (this.headers['content-length'] === false) ? true : false;

// also do not do this anymore //if (skipContentLength) { // delete this.headers['content-length']; //}

// This now comes directly from a stored option if (this.body && !this.skipContentLength) { lines.push("content-length:" + (Frame.sizeOfUTF8(this.body))); }

2) Invert the 'skipContentLength' condition to check for "truthy" values instead of "false". This should simplify the check because undefined, null, NaN, and 0 will all cause the condition to be false and omit the header. Any valid integer values will cause the header to be included.

skipContentLength = !this.headers['content-length']; // do not do this anymore //if (skipContentLength) { // delete this.headers['content-length']; //} `

Thank you.

kum-deepak commented 6 years ago

I will close the same issue in the ng2-stompjs as it belongs to this library.

I am not sure if I understand the issue entirely. The headers object belongs to the Frame object and not to the Client object. Calls that actually generate STOMP frames like connect, send, subscribe etc. accept headers as parameter (see: https://stomp-js.github.io/stomp-websocket/codo/class/Client.html).

Did you try passing content-length as false in every send/publish request (for ng2-stompjs https://stomp-js.github.io/ng2-stompjs/injectables/StompRService.html#publish)?

david-caruso commented 6 years ago

I have tried adding the "content-length": false header to every subscribe and publish call as you suggested. It does not seem to work. I can get it to omit this value during the initial subscribe, but publish frames submitted to the server continue to include this header, even though in the header options I have explicitly told it not to.

kum-deepak commented 6 years ago

I am unable to reproduce your case. Please post a sample code and corresponding console log output.

kum-deepak commented 6 years ago

Planning for next version of stompjs is on the way. Your participation will be really appreciated, please join at https://github.com/stomp-js/stompjs/issues/1