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

Http upload file send is defective when server (jabberd) is configured with max_size: infinity #1222

Closed
emildekeyser opened this issue Apr 10, 2022 · 2 comments · Fixed by #1512
Labels
bug Causing malfunction, data loss and/or heavily degraded user experience good first issue Relatively easy to fix, makes a good start for first time contributors

Comments

@emildekeyser
Copy link

Sending files either doesnt work (after selecting file, you get an error) or uses jingle instead of http upload when both sides are online. This occurs with ejabberd when the max_size parameter from mod_http_upload is set to 'infinity'. Setting it to a arbitray number fixes the issue.

@Yuubi-san
Copy link
Contributor

Yuubi-san commented Mar 11, 2023

The immediate bug seems to be in Dino.Plugins.HttpFiles.HttpFileSender.can_send() at

return file_transfer.size < max_file_sizes[conversation.account];
which doesn't check max_file_sizes[conversation.account] for negativity. Max file size of -1, in turn, comes from Xmpp.Xep.HttpFileUpload.Module.extract_max_file_size() at when the size limit attribute is absent in the XML (which is allowed by the XEP).

The Right Way (TM) to fix this, IMO, is to not signal non-numeric information in-band with numeric information, e.g., return long? instead of long from extract_max_file_size.

@mar-v-in
Copy link
Member

Nullable value types are still rather experimental in Vala (there is a compiler flag --enable-experimental-non-null to add proper nullable checking, but that's still considered experimental). If extract_max_file_size() returned a long? of value null, the comparison for the transfer size would be "valid", but segfault if no further checks are added.

We generally prefer "misbehavior" over segfaults, that's why we try to not use nullable value types (like long?, int?, bool?; nullable reference types do not segfault). That's why I'd prefer to just make extract_max_file_size return long.MAX for unlimited file size.

@fiaxh fiaxh added bug Causing malfunction, data loss and/or heavily degraded user experience good first issue Relatively easy to fix, makes a good start for first time contributors labels Mar 13, 2023
eerielili added a commit to eerielili/dino that referenced this issue Nov 14, 2023
eerielili added a commit to eerielili/dino that referenced this issue Nov 14, 2023
eerielili added a commit to eerielili/dino that referenced this issue Nov 24, 2023
eerielili added a commit to eerielili/dino that referenced this issue Nov 24, 2023
eerielili added a commit to eerielili/dino that referenced this issue Nov 24, 2023
fiaxh pushed a commit that referenced this issue Nov 24, 2023
* Fix for ejabberd XMPP server 'infinity' http upload file size announce

	- fixes #1222

* Update 0363_http_file_upload.vala
mar-v-in pushed a commit that referenced this issue May 9, 2024
* Fix for ejabberd XMPP server 'infinity' http upload file size announce

	- fixes #1222

* Update 0363_http_file_upload.vala
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Causing malfunction, data loss and/or heavily degraded user experience good first issue Relatively easy to fix, makes a good start for first time contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants