search

wandenberg / nginx-push-stream-module

A pure stream http push technology for your Nginx setup. Comet made easy and really scalable.
Other
2.22k stars 295 forks source link

There's no point in unescaping data that is not string #41

Closed karolciba closed 11 years ago

karolciba commented 12 years ago

In our project we're passing around JSON objects. We've got issuses with PushStream library because onmessage callback got broken data as argument - exactly string "[object Object]" - it's happening, because parseMessage function initially converts message from JSON string (with our JSON data object inside) but then make it corrupted by unescaping it.

I've made little patch, unfortunately I wasn't been able to run test, because jshint gem is no longer available. But anyway, I didn't find any tests regarding parsing messages and I don't understand library architecture to write tests from scratch.

Best regards, Karol Ciba

wandenberg commented 12 years ago

Hi Karol,

can you send me the message template you are using and how to reproduce your problem? I understand what you did, but I have to check if will be some side effect on doing this.

About tests, the javascript is uncovered. I'm working on it, and to add documentation to it too.

Thanks for the help.

Regards, Wandenberg

karolciba commented 12 years ago

Hi!

Send messege has format:

{"id":"500516b7639e4029b8000001","type":"Player","change":{"loc":[54.34772390000001,18.5610535],"version":7}}

It's complete JSON object.

Full data received from nginx looks like:

"{"id":31,"channel":"54x19","text":{"id":"500516b7639e4029b8000001","type":"Player","change":{"loc":[54.34772390000001,18.5610535],"version":7}}}

There no quotation marks around my JSON message, becouse I've took it down in message template. Other way I should escape it, because it would be invalid JSON. But then, I think, it would look unusual on backend side - I'd be publishing escaped JSON with no apparent reason.

Push stream message template config: push_stream_message_template "{\"id\":~id~,\"channel\":\"~channel~\",\"text\":~text~}";

To reporoduce message by sending exemplary message and trying to receive it on JavaScript: curl -s -v -X POST 'http://localhost/pub?id=54x19' -d '{"id":"500516b7639e4029b8000001","type":"Player","change":{"loc":[54.34772390000001,18.5610535],"version":7}}'

Please let me know if you have any problems, I'll provide more information.

Best regards, Karol Ciba

2012/7/17 Wandenberg Peixoto < reply@reply.github.com

Hi Karol,

can you send me the message template you are using and how to reproduce your problem? I understand what you did, but I have to check if will be some side effect on doing this.

About tests, the javascript is uncovered. I'm working on it, and to add documentation to it too.

Thanks for the help.

Regards, Wandenberg

Reply to this email directly or view it on GitHub:

https://github.com/wandenberg/nginx-push-stream-module/pull/41#issuecomment-7050526

Karol Ciba :: tel: +48 508288029 :: skype: karol.ciba :: gg: 3365539 :: jabber: karol.ciba@gmail.com :: http://www.karolciba.pl/

rposky commented 12 years ago

I'm curious to know if there's been any negative side effects of the proposed change. I'd love to use it.

wandenberg commented 12 years ago

Hi,

as far as I can see there is no side effects on it. I didn't merge it yet cause I want to add some docs and tests to pushstream.js, and do some more tests on this patch.

Regards, Wandenberg

wandenberg commented 11 years ago

Patch applied. I added some tests to it. Just to be clear, all examples on push stream docs still the same, escaping the text before send. Applications which allow the final users send messages has to pay attention on XSS attacks and properly escape the messages.