crossbario / autobahn-js

WAMP in JavaScript for Browsers and NodeJS
http://crossbar.io/autobahn
MIT License
1.43k stars 228 forks source link

Properly log and react to non-compliant server subprotocol received #231

Open wranai opened 8 years ago

wranai commented 8 years ago

I got this while being connected to the Poloniex socket for a few hours. I only subscribe to the channels, but I don't send anything after that.

I have no idea what caused it, I can't duplicate it, but here it is for if somebody wants to give it a look.

/.../my-project/node_modules/autobahn/lib/transport/websocket.js:99
            var payload = transport.serializer.serialize(msg);
                                              ^

TypeError: Cannot read property 'serialize' of undefined
    at Object.transport.send (/.../my-project/node_modules/autobahn/lib/transport/websocket.js:99:47)
    at self._send_wamp (/.../my-project/node_modules/autobahn/lib/session.js:279:20)
    at Session.join (/.../my-project/node_modules/autobahn/lib/session.js:1156:9)
    at Object.self._transport.onopen (/.../my-project/node_modules/autobahn/lib/connection.js:311:24)
    at WebSocket.<anonymous> (/.../my-project/node_modules/autobahn/lib/transport/websocket.js:118:23)
    at emitNone (events.js:67:13)
    at WebSocket.emit (events.js:166:7)
    at WebSocket.establishConnection (/.../my-project/node_modules/ws/lib/WebSocket.js:887:8)
    at ClientRequest.upgrade (/.../my-project/node_modules/ws/lib/WebSocket.js:778:25)
    at ClientRequest.g (events.js:260:16)

Dependencies, as installed:

┬ autobahn@0.10.1
├─┬ bufferutil@1.2.1
│ ├── bindings@1.2.1
│ └── nan@2.4.0
├── crypto-js@3.1.6
├─┬ msgpack-lite@0.1.20
│ ├── event-lite@0.1.1
│ ├── ieee754@1.1.6
│ ├── int64-buffer@0.1.9
│ └── isarray@1.0.0
├── utf-8-validate@1.2.1
├── when@3.7.7
└─┬ ws@1.1.1
  ├── options@0.0.6
  └── ultron@1.0.2
oberstet commented 7 years ago

cannot reproduce

ghost commented 7 years ago

Maybe someone search the solution of that error: Make sure that you pass right protocols in connection options. https://github.com/crossbario/autobahn-js/issues/87#issuecomment-45862234

bramus commented 3 years ago

The root cause of this issue is that the definition of the serializer on the transport happens automagically by a piece of code that's trying to be smart:

// packages/autobhan/lib/transport/websocket.js
websocket.onopen = function () {
   var serializer_part = websocket.protocol.split('.')[2];
   for (var index in self._options.serializers) {
      var serializer = self._options.serializers[index];
      if (serializer.SERIALIZER_ID == serializer_part) {
         transport.serializer = serializer;
         break;
      }
   }

   transport.info.protocol = websocket.protocol;
   transport.onopen();
}

For the default defined protocols such as wamp.2.json this works fine, but for custom Websocket WAMP implementations such as ocpp1.6 it does not. The value for serializer_part remains undefined, and because of that it cannot find any of the default provided serializers with a matching SERIALIZER_ID.

To work around this issue you can duplicate the existing JSONSerializer and set its SERIALIZER_ID to undefined. That way the code snippet above will find a match, and everyting will work fine.

Like so:

// This an exact copy of the existing JSONSerializer, but with SERIALIZER_ID set to `undefined`
// This trick will fool the serializer detection code, enabling support for custom protocols
class JSONSerializer {
    constructor(replacer, reviver) {
        this.replacer = replacer;
        this.reviver = reviver;
        this.SERIALIZER_ID = undefined;
        this.BINARY = false;
    }

    serialize(obj) {
       try {
          if (!obj) return '';
          var payload = JSON.stringify(obj, this.replacer);
          return payload;
       } catch (e) {
          console.warn('JSON encoding error', e);
          throw e;
       }
    }

    unserialize(payload) {
       try {
          if (!payload) return {};
          var obj = JSON.parse(payload, this.reviver);
          return obj;
       } catch (e) {
          console.warn('JSON decoding error', e);
          throw e;
       }
    }
}

const connection = new autobahn.Connection({
    url, 
    realm,
    protocols: ['ocpp1.6'], // Inject custom protocols
    serializers: [new JSONSerializer()], // Inject our “custom” JSONSerializer into Autobahn
});

Note that this workaround won't work with custom protocols that contain two dots in their identifier.

oberstet commented 3 years ago

for custom Websocket WAMP implementations such as ocpp1.6 it does not

not sure I understand, as far as I get it from a quick glance at OCPP, it can run over WebSocket, but is otherwise unrelated to WAMP.

if that is true, it doesn't make sense to direct AutobahnJS at a OCPP endpoint ..

if, on the other hand, there actually is a WAMP binding or sth for OCPP, then using a ocpp1.6 websocket subprotocol identifier in a WAMP connection is a protocol error (the WAMP spec exactly lists the valid identifiers)

bramus commented 3 years ago

In the OCPP-J specification (which is an addendum to the OCPP spec itself), it's stated that they built a protocol that is similar to WAMP. They support CALL, CALLRESULT, and CALLERROR. It looks that it's right in line with the current WAMP spec, so AutobahnJS can be used.

About that ocpp1.6 specifier … while testing it became clear that their implementation is based upon the WAMP 1. This is not explicitly stated in their spec — I had to derive this from the the messagetypenumbers used — which is something that I'll address with them.


Winging back to the initial issue of the OP: the clue remains that no serializer can be created for the defined protocol. This can be due to a faulty protocol (cfr. what I had) or due to a suffix that has no matching serializer registered.

Perhaps an extra check with its own exception in websocket.onopen would come in handy? That way the mysterious error as described by the OP can be replaced by a descriptive one.

oberstet commented 3 years ago

while testing it became clear that their implementation is based upon the WAMP 1. This is not explicitly stated in their spec — I had to derive this from the the messagetypenumbers used — which is something that I'll address with them.

aahh! thanks for digging=)

couple of comments from my side:

I wasn't aware of their "use" of WAMPv1 patterns or protocol elements at all. usually, implementors of WAMP get in touch with us at

https://github.com/wamp-proto/wamp-proto/ https://gitter.im/wamp-proto/wamp-proto https://forum.crossbar.io/

If they had, I would have strongly urged them to use WAMPv2, and to not reinvent wheels and simply put the OCPP application layer semantics on top of native, unmodified WAMPv2.

Eg with WAMPv2 you could securely call into a procedure registered in a charge point from the cloud. Via NATs, on top of standard WebSocket. You cannot do that with WAMPv1.

Anyways: if you get in touch with them, why not hint at ^? and of course, the general fact that it is confusing and bad for users to implement a non-standard compliant variant of WAMP.

btw: WAMP (both v1 and v2) are open: anyone can take parts, steal, modify, etc - BUT: if the result is not WAMP (v2!) compliant, then we ask not to reuse our protocol name "WAMP". the reason exactly is to prevent confusing our users by half baked, non-WAMP things that claim to be "WAMP". pls see

https://wamp-proto.org/faq.html#why-is-wamp-trademarked


Perhaps an extra check with its own exception in websocket.onopen would come in handy?

yes, I agree. it would be good to detect that, log a proper message, and then deny connecting (from client side). changing the labels of this issue accordingly.

bramus commented 3 years ago

Thanks for the feedback + adjustments to this issue, @oberstet!


Regarding OCPP: do know that they're not using the name WAMP (they use OCPP-J) or state that they have implemented it. They're simply referring to WAMP as a source of inspiration to solve the same problem WAMP solves. Here's the relevant passage from their docs:

To encode these request/response relations we need a small protocol on top of WebSocket. This problem occurs in more use cases of WebSocket so there are existing schemes to solve it. The most widely-used is WAMP but with the current version of that framework handling RPCs symmetrically is not WAMP compliant. Since the required framework is very simple we decided to define our own framework, inspired by WAMP, leaving out what we do not need and adding what we find missing.

Basically what we need is very simple: we need to send a message (CALL) and receive a reply (CALLRESULT) or an explanation why the message could not be handled properly (CALLERROR). For possible future compatibility we will keep the numbering of these message in sync with WAMP. Our actual OCPP message will be put into a wrapper that at least contains the type of message, a unique message ID and the payload, the OCPP message itself.

Their first work on this started back in 2012 — back when WAMP v1 was a thing — so it could perfectly be that the passage “handling RPCs symmetrically is not WAMP compliant” is no longer valid.

(Apologies for hijacking this thread with this info)

oberstet commented 3 years ago

^ thanks for clarifying! that use of "WAMP" is completely fine!

well, technically, I still think OCPP could likely be put on top of standard WAMPv2, and that the resulting stack would be "better" ... but that is a different matter (just my opinion). Eg with WAMPv2 an Android app (eg AutobahnJava based) could do a routed WAMP RPC into a charging point exposing the RPC endpoint. Both the Android thing, and the charging point sit behind NATs and do not have any listening ports open. Yet the RPC is properly routed via Crossbar.io in cloud. All WAMP clients can do all 4 actions (pub, sub, call, register).