iiegor / xat-server

A Node.js private server of xat.com
6 stars 2 forks source link

30.01.16-31.01.16 changelist #17

Closed huumanoid closed 8 years ago

huumanoid commented 8 years ago

Time to discuss.

Changes:

1. Private message handling.

The difference between my and your version that my version dispatch both 'z' and 'p' packets. It seems that your version contains bug. What if global.Server.getClientById(toID) is undefined? Use

global.Server.getClientById(toID)? 

instead. Question: are we need to emit 'z' private message packages at all? I think it's ok if we renounce 'z' or 'p' package. 'z' packages is more preferable it's format is more clear. Look at the formats in all cases: <p u="FROM-USER_ID" t="MESSAGE" [s="2" d="FROM-USER_ID"] /> - user receives

- user sends (pm)

- user sends (pc) - user receives and sends ## 2. Multichat Now user can log in several chats. I suggest new Server scheme ![img](http://i.imgur.com/8p7Qd1j.png) It's clear about rooms, what do clients array means now? Clients array points to id's handlers, which are latest active. For example: my id is 1337 and i'm connected to chats 15 and 700 i've posted a message in 15. So, my last active chat is 15 and clients[1337] should points to handler with chat == 15 and then i've tickled user at chat 700. Now my latest active chat is 700 and clients[1337] should points to handler with chat == 700 I'm going to implement private message routing soon. ## 3. Prepared statements. In current realisation, sql injections are possible (and it's easy to perform them) For example: use userno = "123' OR '1'>'2" to log in as user with id 123 Prepared statements helps to avoid this situation. That's it! ## 4. Dup fix? We can use my or your version, i think it doesn't matter.

iiegor commented 8 years ago

Ok, this looks pretty fine. I think all the minimal mistakes are fixed and for the moment what you said on this comment looks :+1: .

So to finish this can you please resolve the git conflicts?

huumanoid commented 8 years ago

Thank you for review! Finally i'll rename database.execp to exec. While resolving conflicts, i will choose my private message handling and your dupfix.

iiegor commented 8 years ago

Merged, thank you for your contributions :dizzy: