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

Conversation doesn't close the connection for file transfers over S5B #3500

Open
hrxi opened this issue Jul 22, 2019 · 4 comments

Comments

@hrxi
Copy link

commented Jul 22, 2019

Unlike the IBB, file transfers over socks5 do not close the stream when the file is completely sent. This seems like an inconsistency.

I don't know how exactly one is supposed to detect that a file transfer is finished, but "peer closed the connection" and "peer send size bytes" seem viable ones, Conversation seems to implement the latter one and is thus does not run into problems with itself.

General information

  • Version: 2.5.4+fcr
  • Device: Nexus 5X
  • Android Version: Android 8.1 (LineageOS 15.1)
  • Server name: from jabber.ccc.de to 5222.de
  • Server software: not known
  • Installed server modules: not known
  • Conversations source: F-Droid

Steps to reproduce

  1. Open a file transfer to a target that supports S5B.
  2. Wait for the file transfer to finish.

Expected result

Socks5 connection is closed.

Actual result

Socks5 connection remains open, waiting for the receiver to terminate the Jingle session.

Debug output

Not provided, can add if wanted.

@iNPUTmice

This comment has been minimized.

Copy link
Member

commented Jul 22, 2019

Can you explain why you think it is a problem to wait until the recipient has signaled that they received the file?

@hrxi

This comment has been minimized.

Copy link
Author

commented Jul 25, 2019

It's an inconsistency between the behavior of in-band bytestreams and s5b.

I'm not entirely sure whether the behavior of IBB or s5b is the correct one. Closing the (IBB/SOCKS5, not Jingle) connection after the file is completely sent gives the receiver an easy way to check whether the file has been completely sent.

@iNPUTmice

This comment has been minimized.

Copy link
Member

commented Jul 25, 2019

It's probably a bit problematic that IBB is closing the stream. But you should be counting bytes anyway if only to ensure that the file isn’t larger than advertised.

@hrxi

This comment has been minimized.

Copy link
Author

commented Aug 6, 2019

(In Dino, the file end is now also determined by byte counting.)

Still an inconsistency, don't know which of "close the connection" or "don't close the connection" should be the way to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.