search

synac-chat / synac-legacy

Some IRC-like chat application that might become good one day
11 stars 1 forks source link

Disallow non-utf8 (binary) data? #17

Closed jD91mZM2 closed 6 years ago

jD91mZM2 commented 6 years ago

Reason I added this in the first place is because I originally intended any message to be able to be E2E encrypted. But now when we're making them only work in DMs (because only one person can decrypt them anyways), should I revert it? I mean, that will probably reduce packet size (not sure though, this is MsgPack, not JSON...).

ttofis commented 6 years ago

Not sure I understand what you mean.

jD91mZM2 commented 6 years ago

In programmer's terms: Should I store shit in a string or byte array?

ttofis commented 6 years ago

Definitely string. Byte array adds JUST another byte to string conversion really.

tbodt commented 6 years ago

use byte array. flexibility is good.

jD91mZM2 commented 6 years ago

I just hope MsgPack byte arrays aren't as inefficient as in JSON.

tbodt commented 6 years ago

https://github.com/msgpack/msgpack/issues/121

jD91mZM2 commented 6 years ago

Oh god. Hope that doesn't make injections possible...

tbodt commented 6 years ago

yeah maybe we should just use utf8

~Theodore

On Oct 13, 2017, at 9:30 AM, jD91mZM2 notifications@github.com wrote:

Oh god. Hope that doesn't make injections possible...

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

tbodt commented 6 years ago

we can always expand it in the future

jD91mZM2 commented 6 years ago

But I still need to use a byte array where encryption is possible. So I need to know if injections are possible D:

tbodt commented 6 years ago

fuk

~Theodore

On Oct 13, 2017, at 9:32 AM, jD91mZM2 notifications@github.com wrote:

But I still need to use a byte array where encryption is possible. So I need to know if injections are possible D:

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

jD91mZM2 commented 6 years ago

Just tried converting JSON to MsgPack in the online thingy. It seems like it adds the length in front of the array. good.

jD91mZM2 commented 6 years ago

Vote up for string, vote down for byte array.

ttofis commented 6 years ago

Not sure tbh... It may cause issues with certain implementations of msgpack, idk. Well, keeping it how it is rn is a bad option?

jD91mZM2 commented 6 years ago

It's not a bad option at all. Might be a little tedious having to convert it to a String for Rust users, but nothing big. All is fine!

ttofis commented 6 years ago

Well think of me with the go lib and understand how tedious all this is. Since its already pretty much fine, this shouldn't be what's holding you to release