Flotype / now

NowJS makes it easy to build real-time web apps using JavaScript
http://www.nowjs.com
MIT License
1.91k stars 175 forks source link

clients should not be able to change data on the server without validation; force server-side "setters" #44

Closed tommedema closed 13 years ago

tommedema commented 13 years ago

Any data that is to be used on the server (and all these variables are, because otherwise they should not be in a synced magic pool) should be verified.

For example, the current implementation allows clients to set huge amounts of data to a single variable, causing memory issues on the server side.

Thus, clients should only be able to change a synced variable by calling a setter on the server side. Eg.:

now.setUserName("new name"); rather than now.userName = "new name";

This also makes sure that the new variable can be used immediately.

The server's setUserName method will be called, in which the server can validate the data and then set the actual variable on the server side:

this.now.userName = userName;

ericz commented 13 years ago

Hey Tom,

We've added the clientWrite option which allows you to stop clients from writing to the server.

 nowjs.initialize(httpServer, {clientWrite: false});

However changes are rejected on the server side so it is important to make sure that any type of data sent to the server is rejected if it surpasses a certain size

balupton commented 13 years ago

Sweet, it does seem like a big security issue with having clientWrite as true.

So with:

nowjs.initialize(httpServer, {clientWrite: false});

What's the best practice equivalent of:

now.receiveMessage = function(name, message){
  $("#messages").append("<br>"+name+": "+message);
}

I couldn't get the following to work:

Server:

/**
 * Meet the client
 * @return {integer} id
 */
everyone.now.meet = function(options,_callback){
    // Apply Options
    if ( typeof options.notify === 'function' ) {
        this.now.notify = options.notify;
    }

    // Next
    _callback(this.now.id);
};

// ...
everyone.now.notify(n);

Client:

// Now
window.now.ready(function(){
    // Bind Now
    window.now.meet(
        // Options
        {
            notify: function(_state){
                if ( _state !== me.currentState ) {
                    me.reset();
                }
            }
        },
        // Callback
        function(_id){
            // Apply Id
            document.title = me.id = _id;

            // Init Sync
            me.reset();
        }
    );
});

Returns error on server:

TypeError: Object #<Object> has no method 'notify'
    at Object.<anonymous> (/Users/balupton/Dropbox/Server/public_html/sites/nowpad/server.js:180:16)
    at Object.<anonymous> (native)
    at Object.remoteCall (/usr/local/lib/node/.npm/now/0.5.2/package/lib/nowServerLib.js:49:19)
    at [object Object].<anonymous> (/usr/local/lib/node/.npm/now/0.5.2/package/lib/nowServerLib.js:173:46)
    at [object Object].emit (events.js:64:17)
    at [object Object]._onMessage (/usr/local/lib/node/.npm/socket.io/0.6.17/package/lib/socket.io/client.js:58:10)
    at Parser.<anonymous> (native)
    at Parser.emit (events.js:64:17)
    at Parser.parse (/usr/local/lib/node/.npm/socket.io/0.6.17/package/lib/socket.io/transports/websocket.js:195:12)
    at Parser.add (/usr/local/lib/node/.npm/socket.io/0.6.17/package/lib/socket.io/transports/websocket.js:182:8)
ericz commented 13 years ago

@balupton

Hmm I believe there is an outstanding bug with nested functions being passed as parameters. So options.notify is not being actually passed the server

balupton commented 13 years ago

@ericz

Yeah, passing the notify function as an argument rather than inside an object worked great. Would you like me to file a bug for sending objects to the server?

ericz commented 13 years ago

Yep that'd be very helpful, thanks!

On Sun, Apr 24, 2011 at 8:32 PM, balupton < reply@reply.github.com>wrote:

@ericz

Yeah, passing the notify function as a argument rather than inside an object worked great. Would you like me to file a bug for sending objects to the server?

Reply to this email directly or view it on GitHub: https://github.com/Flotype/now/issues/44#comment_1051668

510-691-3951 EECS Student at UC Berkeley http://ericzhang.com