Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature request] Allow sending messages with (nearly) unlimited size #492

Open
sudden6 opened this issue Mar 3, 2017 · 48 comments
Open
Labels
messenger Messenger P3 Low priority suggestion Suggestions
Milestone

Comments

@sudden6
Copy link

sudden6 commented Mar 3, 2017

Currently every Tox client must split long messages into smaller parts. I think it would be beneficial to the user experience and the developer experience if toxcore supported messages with unlimited size or with sizes that are way longer than the current maximum (1372 bytes).

I think this can be implemented either by introducing a flag that indicates a message as part of a multipart message.

@sudden6 sudden6 added suggestion Suggestions messenger Messenger labels Mar 3, 2017
@yurivict
Copy link
Member

yurivict commented Mar 4, 2017

I almost implemented this in qTox.
This requires persistent storage that toxcore doesn't have.

@aaannndddyyy
Copy link

+1

@sudden6
Copy link
Author

sudden6 commented Mar 4, 2017

@yurivict I'm interested in how you implemented this in qTox, anything public yet?

Also how do you keep compatibility to other clients?

I also think it can be done without persistent storage, my simple approach would be to only mark a message as confirmed delivered when all parts have arrived, but to retransmit parts as long as both clients are online.

@yurivict
Copy link
Member

yurivict commented Mar 5, 2017

@sudden6 If the messages aren't too large, it is ok to defragment without persistent storage. But imagine that somebody will send 1GB message? He may need to reboot the computer in between, and previous parts will get lost and he will need to start all over again. It is better to have persistent storage IMO in the general case.

I will release qTox with defragmenter once I will work out few remaining problems.

I also think that it is better to have a mid-level library, xtox, that will contain defragmenter itself to allow other clients to use it. This assumes that all clients use sqlite. I am not sure if this is a reasonable assumption.

This xtox library could export another version of Tox interface that will allow large messages.
Bug #499 is a pre-requisite for such library.

@isotoxin
Copy link

isotoxin commented Mar 9, 2017

Isotoxin has long able to send messages of unlimited size.

I almost implemented this in qTox.

It will be good to make it compatible together, but I understand it is impossible.

@yurivict
Copy link
Member

yurivict commented Mar 9, 2017

I am going to release the library libtox-defragmenter.so that will do message defragmentation. I am polishing the details now.

This shouldn't be done in the clients, because then defragmentation isn't going to be compatible between clients.

@yurivict
Copy link
Member

yurivict commented Mar 11, 2017

Tox-defragmenter library: https://github.com/yurivict/tox-defragmenter

It needs:

qTox compatible with defragmenter is in my branch: https://github.com/yurivict/qTox/tree/tox-defragmenter

I will likely update tox-defragmenter based on further testing.

TODO:
Make client aware of the persistent nature of receipts returned by tox-defragmenter. Table faux_offline_pending will need the field persistent_receipt which will make qTox show that the message isn't yet sent, but will prevent it from resending it.

@sudden6
Copy link
Author

sudden6 commented Mar 12, 2017

I don't think tox-defragmenter should come with it's own db, most clients already have a db or something else to store persistent data.

In your use case it fits perfectly for qTox, but what about more lightweight clients that don't use sqlcipher?

@isotoxin can you describe the approach you used?

@yurivict
Copy link
Member

yurivict commented Mar 12, 2017

I don't think tox-defragmenter should come with it's own db, most clients already have a db or something else to store persistent data.

It doesn't come with db. It uses the db supplied by the client.

In your use case it fits perfectly for qTox, but what about more lightweight clients that don't use sqlcipher?

The persistent storage is needed to handle large messages. It can be made to not explicitly depend on sqlcipher, only require the functions, so that both sqlcipher and SQLite will fit.

@sudden6
Copy link
Author

sudden6 commented Mar 12, 2017

AFAICT your code directly depends on sqlcipher or SQLite, but what if a client wants to use (encrypted) text files as storage?

I didn't look to deep into your code, but what happens if a "normal" client receives a message with your splitting protocol?

@yurivict
Copy link
Member

AFAICT your code directly depends on sqlcipher or SQLite, but what if a client wants to use (encrypted) text files as storage?

I added the ability to use the in-memory database: tox_defragmenter_initialize_db_inmemory().

I didn't look to deep into your code, but what happens if a "normal" client receives a message with your splitting protocol?

They will still show it as split, as it happens today.

@yurivict
Copy link
Member

@sudden6

Here's what I did:

  • Added the ability to use internal data structures for clients that don't use sqlite or sqlcipher.
  • Added the test suite to https://github.com/yurivict/tox-defragmenter that currently runs stress tests. Will add more tests later.

Now any client is able to use defragmenter.

Do you think the dependency pull requests for toxcore and qTox can be merged?
Once they are merged, I can create the pull request for qTox with the option to enable long messages.

I would like to add the file attachment feature in qTox, like in e-mail. It will be able to send file attachments in MIME format. But I can't proceed with this without the ability to send large messages.

@GrayHatter
Copy link

@yurivict You want message splitting in toxcore so you can send files?

@yurivict
Copy link
Member

yurivict commented Mar 17, 2017

I would like to send large messages to support various other features, like attachments.
IMO, it's better to keep toxcore simple. Toxcore lacks persistent storage, without which it is hard to send large messages over slow networks.

@GrayHatter
Copy link

you know toxcore has a file transfer API right?

As well as generic lossless packets?

@yurivict
Copy link
Member

Lossless packets are limited in size by TOX_MAX_CUSTOM_PACKET_SIZE=1373.
There are benefits that defragmenter has:

  • It extends API to seamlessly support arbitrarily long packets.
  • Long packet transfers are persistent over client restarts.

I don't think file transfers can be continued after client restart.

@GrayHatter
Copy link

uTox has no problem restarting an in-progress file transfer. Through network failures, client restarts, etc...

https://github.com/TokTok/c-toxcore/blob/master/toxcore/tox.h#L1968-L1979

@yurivict
Copy link
Member

yurivict commented Mar 17, 2017

The way I am proposing is client-neutral. It doesn't even require client to do anything special. How can uTox way be extended to all clients? Sending long message over the file transfer requires the special code that does this in each client.

Also, if lossless packets and file transfers were the same, why this issue is even here? Obviously, it is a problem.

The way I am proposing is through the API adapter. It's client-neutral and toxcore-neurtal. No need to add code in toxcore.

@GrayHatter
Copy link

The issue is here because qTox sucks?

No, not really. But I don't know why qTox can't resume file transfers. And while I agree that tox should do message splitting for the clients. If you're sending SO MUCH TEXT that you need an SQL database to send it. Perhaps via the Tox Message API is the wrong way?

Maybe a file/document?

Sure you COULD send files and such via tox messages, but if your solution to doing so is; interface with a SQL database, instead of the existing file transfer API. I have to wonder WHY?

@yurivict
Copy link
Member

yurivict commented Mar 17, 2017

I am not sending files, I am extending messaging to send e-mails. File sending is a different feature. My approach avoids splitting messages, and avoids adding complexity to clients and toxcore.

Practically speaking, how can long messages be currently implemented? Every client needs to implement them through file transfers/lossless packets. They need to keep track of the state, to restart from the point when interrupted, etc. With my approach they do not need to do anything above of what they do now. So what is your objection?

@GrayHatter
Copy link

But toxcore is an instant messenger, not an email client. And IMO, shouldn't try to be both. Do one thing, and do it really well.

@yurivict
Copy link
Member

yurivict commented Mar 17, 2017

Why can't it be both? It is convenient to send files through the same path as short messages are sent.

@GrayHatter
Copy link

Why can't cars make coffee? The use case for an instant messenger is different from email.

@yurivict
Copy link
Member

Message can be short or long. Why do you want to limit what Tox can do? The more features the better. IM isn't supposed to do video too.

@yurivict
Copy link
Member

The system becomes great when it has ecosystem, and is feature-rich. It's not good to limit features of Tox, IMO.

@optimumtact
Copy link

Tox's stated goal is to be able to replace skype in users lives, anything outside that area would probably be best suited for a fork that can better cater to a protocol like meail

@yurivict
Copy link
Member

Asking for forks is a death of the project. Which fork users should use then, or devs contribute to? There should be one strong system. Why can't we innovate above of what skype can do? Is skype a golden standard? I am for innovation.

@yurivict
Copy link
Member

yurivict commented Mar 17, 2017

Many people are crazy about the Slack IM. Slack supports attachments, directory and conversation sharing, etc. Why we should model skype, and not slack?

@Chuongv
Copy link

Chuongv commented Mar 17, 2017

That analogy is poor. A more correct analogy is that you are suggesting to convert a already made product (Skype) into Slack which is entirely different.

@yurivict
Copy link
Member

yurivict commented Mar 17, 2017

into Slack which is entirely different.

The features aren't mutually exclusive.

Why I can send a 3 sentence message, and can't send a 5 page article? It doesn't make sense to me. Larger size messaging is a natural expectation. Who decides what is IM and what isn't? Is there a definition anywhere?

I asked friends, they said they didn't realize the message size is limited. They asked "Why is the size limited?" The intuitive expectation is that sizes aren't limited.

@optimumtact
Copy link

forks are not the death of a project? they're a healthy part of the open source ecosystem.

@Diadlo
Copy link

Diadlo commented Mar 17, 2017

Why is the size limited?

@yurivict Yes, size of the message is limited. But long message can be splitted on few parts and sended separately. AFAIK, it's how most clients do it now.
ADDED: And I don't understand, what's wrong with this solution?

@yurivict
Copy link
Member

es, size of the message is limited. But long message can be splitted on few parts and sended separately. AFAIK, it's how most clients do it now

@Diadlo I know. That's the problem I am trying to solve (actually, I did solve it). But people here seem to think that limited message size is a feature, and not a problem, and shouldn't be solved.

@Diadlo
Copy link

Diadlo commented Mar 17, 2017

@yurivict What the problem with auto-split?

@yurivict
Copy link
Member

yurivict commented Mar 17, 2017

One problem is that when the larger article gets split, it can't be easily coped-pasted in the same form. Timestamps get copied with it (in qTox). Timestamps shouldn't be added in the middle of the message.

Another problem is that such split breaks paragraphs.

Anther problem is that such split limits many further features. For example, I want to have file attachments. They aren't possible with split.

Another problem is that splitting isn't a natural or intuitive behavior. It's a technical glitch. I think the limit is in part due to UDP.

@yurivict
Copy link
Member

Skype has the limit of 800 characters for the first message, and 8000 characters for the subsequent messages.

In this recent thread skype users ask skype to eliminate this message limit: https://www.skypefeedback.com/forums/299913-generally-available/suggestions/13124298--message-is-to-long-to-send

@Diadlo
Copy link

Diadlo commented Mar 17, 2017

Just small IMO: It's bad idea send message over 8k characters through the chat

@sudden6
Copy link
Author

sudden6 commented Mar 17, 2017

I agree with @yurivict that the limit for one message is too small. I don't think the limit should be several GB, but ~10K would be nice to not have sudden breaks in long messages.

For the emali idea, just send it over existing filetransfer.

@yurivict
Copy link
Member

yurivict commented Mar 17, 2017

I like when several files are attached to the message, not sent separately, one by one. I would gladly use e-mail, but everybody knows that e-mail is insecure. I don't understand why does the limit exist.

@GrayHatter
Copy link

GrayHatter commented Mar 17, 2017 via email

@sudden6
Copy link
Author

sudden6 commented Mar 18, 2017

IMO this problem could be better solved by extending Messenger with a new packet type, e.g. MESSAGE_MULTI.

This new packet type would then contain a header with fragment number and eventually some other metadata.

The receiver will acknowledge the arrival of every fragment of the message and if all parts arrived acknowledge the whole message. If one of the clients goes offline during the transfer, it's the responsibility of the sender to re-transmit the message when both parts are online again. Fragments are only stored in memory.

The tox_friend_send_message function would decide based on the message length if a multi part message is needed.

PROS

  • no api change, the current API has no size limit, clients using TOX_MAX_MESSAGE_LENGTH correctly will just start to send long messages
  • no persistent storage needed
  • no addon libs needed

CONS

  • Probably not good for very large (several MB) messages, because every fragment needs to get re-transmitted on connection drops
  • @zetok brought up the possibility of an Out of Memory DoS attack

CHALLENGES

  • Fallback for clients that don't yet have this feature? Implement client capability packet #513 would also help
  • Ensure integrity of the whole message -> probably a good idea to include a hash over the whole message

I think such a feature will be very a huge win for Tox, with not much cost to implement on the existing infrastructure.

@yurivict
Copy link
Member

yurivict commented Mar 18, 2017

@sudden6

no api change, the current API has no size limit

But the limit is returned by tox_max_message_length() (same as TOX_MAX_MESSAGE_LENGTH).

CONS
Probably not good for very large (several MB) messages

^ yes. I don't understand your resistance to the persistent storage. Without it larger messages will suffer progressively. There is no way around it without the storage.

IMO this problem could be better solved by extending Messenger with a new packet type, e.g. MESSAGE_MULTI.

Why have two types of messages based on size? Doesn't this unnecessarily complicate the API?

@sudden6
Copy link
Author

sudden6 commented Mar 18, 2017

But the limit is returned by tox_max_message_length() (same as TOX_MAX_MESSAGE_LENGTH).

yeah, that's why the API isn't changed, on new toxcore versions this would just return a higher value.

I don't understand your resistance to the persistent storage. Without it larger messages will suffer progressively. There is no way around it without the storage.

For very large messages there's already a better solution IMO, namely file transfers. The goal of my suggestion is to raise the limit of how big a text message can be, not to transfer files as or huge amounts of data.

Why have two types of messages based on size? Doesn't this unnecessarily complicate the API?

The packet type is only toxcore internal, clients will not know how the message is sent and only get a pointer to the message + it's size like it is now.

@isotoxin
Copy link

IMO this problem could be better solved by extending Messenger with a new packet type, e.g. MESSAGE_MULTI

Agree.
But my suggestion is just set limit of message length to 64kbytes.
That it. No one needs a message size greater than 64kb. Why more?
Btw, Isotoxin's message input control is limited to 8kb. Even this is enough for everyone.

@sudden6
Copy link
Author

sudden6 commented Mar 18, 2017

@isotoxin yes, I agree that a limit is needed, 64KB seems reasonable. Does isotoxin also use another packet type for large messages?

@yurivict
Copy link
Member

No one needs a message size greater than 64kb. Why more?
... 8kb. Even this is enough for everyone.

You are factually wrong. I need messages more than 8kB.
And in this thread hundreds of users said that they need longer messages too: https://www.skypefeedback.com/forums/299913-generally-available/suggestions/13124298--message-is-to-long-to-send

@Diadlo
Copy link

Diadlo commented Mar 18, 2017

@yurivict Again: tox doesn't block long message. You will never have a error like: "message to long"
But really looong message open way to "Out of Memory DoS attack"

@isotoxin
Copy link

Does isotoxin also use another packet type for large messages?

It uses same message packet id, but it appends special marker ("\1\1") at end of message, if it's size is bigger then current limit. Even target client does not support "message chain" capability, it receive correct chain of messages.
If target client support "message chain" capability, and if it detected special marker at end of message, it just wait for 5 seconds and append next messages to marked one.

@iphydf iphydf added this to the v0.3.0 milestone Jan 16, 2018
@iphydf iphydf added the P3 Low priority label Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
messenger Messenger P3 Low priority suggestion Suggestions
Development

No branches or pull requests

9 participants