stomp-js / stompjs

Javascript and Typescript Stomp client for Web browsers and node.js apps
Apache License 2.0
764 stars 81 forks source link

Workaround - React Native STOMP issue because NULL chopping #55

Open omars94 opened 5 years ago

omars94 commented 5 years ago

I was facing an issue where the stomp client doesn't get connected to server because of the null terminator and the connect message wasn't parsed at server side correctly because it was sent as string instead of binary array.

I did lots of debugging on it and the solution was to make isBinaryBody always true like so

FrameImpl.prototype.serialize = function () {
        var cmdAndHeaders = this.serializeCmdAndHeaders();
        if (this.isBinaryBody || true) { // to always return as binary array
            return FrameImpl.toUnit8Array(cmdAndHeaders, this._binaryBody).buffer;
        }
        else {
            return cmdAndHeaders + this._body + byte_1.BYTE.NULL;
        }
    };
kum-deepak commented 5 years ago

Many thanks for this. I do not use React Native myself.

This was earlier reported as slightly different issue (https://github.com/stomp-js/stomp-websocket/issues/37). The underlying issue seems to be Reactive Native having issue with NULL characters in strings. NULLs in string are needed to support STOMP protocol. See: https://github.com/facebook/react-native/issues/12731

Based on your analysis it seems, sending messages in binary bypasses the issue. I will wait for your results - based on that an option (say alwaysSendBinaryPackets) may be added to the config.

Making this change will control packets sent to the broker. However the communication may fail if the server sends a text packet. Some brokers (like RabbitMQ has an option to always send binary packets).

kum-deepak commented 5 years ago

A flag is added in configuration forceBinaryWSFrames, this will make outgoing frames to be binary.

omars94 commented 5 years ago

was it added to the package yet? I couldn't find this change in change-log

omars94 commented 5 years ago

Tried it and its working perfectly.

WhiteHatTux commented 5 years ago

Using only binaryMessages was causing other problems for me, so I found this to handle the error on the spring server:

package com.beskgroup.dst.backend.config;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.web.socket.TextMessage;
import org.springframework.web.socket.WebSocketHandler;
import org.springframework.web.socket.WebSocketMessage;
import org.springframework.web.socket.WebSocketSession;
import org.springframework.web.socket.handler.WebSocketHandlerDecorator;

/**
* Register this in WebSocketConfig
*/
public class CustomWebSocketHandlerDecorator
  extends WebSocketHandlerDecorator {

  private static final Logger logger = LoggerFactory.getLogger(CustomWebSocketHandlerDecorator.class);

  public CustomWebSocketHandlerDecorator(WebSocketHandler delegate) {
    super(delegate);
  }

  @Override
  public void handleMessage(final WebSocketSession session, final WebSocketMessage<?> message)
    throws Exception {
    if (message instanceof TextMessage) {
      TextMessage msg = (TextMessage) message;
      String payload = msg.getPayload();

      // only add \00 if not present (iOS / Android)
      if (!payload.substring(payload.length() - 1).equals("\u0000") && !payload.equals("\n")) {
        final TextMessage message1 = new TextMessage(payload + "\u0000");
        super.handleMessage(session, message1);
        return;
      }
    }

    super.handleMessage(session, message);
  }
}

The register this decorator with your websocket:

public class WebSocketConfig
  extends AbstractWebSocketMessageBrokerConfigurer {

  @Override
  public void configureWebSocketTransport(WebSocketTransportRegistration registration) {
    registration.addDecoratorFactory(CustomWebSocketHandlerDecorator::new);
  }
  ....
} 
slavikdenis commented 5 years ago

@WhiteHatTux hi, we tried your implementation without any luck :/

kum-deepak commented 5 years ago

I think it may be worth trying to fix the underlying issue in React Native :smile:

slavikdenis commented 5 years ago

@kum-deepak yeah that would be better 👍

kum-deepak commented 5 years ago

https://stomp-js.github.io/workaround/stompjs/rx-stomp/ng2-stompjs/2019/06/10/react-native-null-chopping-issue.html