Bug 46783

Summary: [next] Fix FileTransfer ContentHash property
Product: Telepathy Reporter: Jonny Lamb <jonny.lamb>
Component: tp-specAssignee: Telepathy bugs list <telepathy-bugs>
Status: RESOLVED MOVED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: normal    
Priority: medium CC: xclaesse
Version: git master   
Hardware: Other   
OS: All   
URL: http://cgit.freedesktop.org/~jonny/telepathy-spec/commit/?h=ft-46783
Whiteboard:
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 68661    

Description Jonny Lamb 2012-02-29 09:54:32 UTC
Now that we've got a next branch, we can fix this:

FileTransfer.ContentHash is really annoying because it means the person requesting the channel must be the one who hashes the file.

We should fix this by:

 1. specifying that the file send request is sent when ProvideFile() is called, and not when the new channel is created.

 2. making ContentHash and ContentHashType mutable *before* ProvideFile() has been called (just like the URI property). Of course for incoming file transfers, it's always immutable.

So here is the outgoing file transfer case:

 1. request a FileTransfer channel in some dummy app with:

  {
     ChannelType: FileTransfer,
     TargetHandleType: CONTACT,
     TargetID: "foo@bar.com",
     URI="file:///home/user/party.jpg"
  }

 2. the appropriate channel appears and another app is activated as the handler. The handler sees the file is /home/user/party.jpg and hashes it to get the hash "abcdefghijklmnopqrstuvwxyz". So calls:

  Set("FileTransfer", "ContentHashType", 1337)
  Set("FileTransfer", "ContentHash", "abcdefghijklmnopqrstuvwxyz")

 3. the handler then calls ProvideFile().

 4. the CM then sends the, say, SI file transfer stanza.

The incoming transfer case is pretty much the same. How does this sound then?
Comment 1 Simon McVittie 2012-03-01 04:55:59 UTC
(In reply to comment #0)
> How does this sound then?

Entirely reasonable, if I'm interpreting it correctly:

> The incoming transfer case is pretty much the same

I assume you mean "... the same as it always was" rather than "the same as the outgoing case"? ContentHashType is chosen by the sender, and ContentHash is computed by the sender, surely? (Including "sender does not send a hash", which you could view as a sort of trivial choice/computation.)

I think the semantics of CH, CHT are sort of the opposite of URI: you can meaningfully set URI right up until you Accept an incoming file transfer, but not afterwards, and it's immutable on outgoing FTs; conversely, you can meaningfully set CH, CHT right up until you Provide an outgoing FT, but they're immutable on incoming FTs.
Comment 2 Jonny Lamb 2012-03-01 08:18:24 UTC
(In reply to comment #1)
> Entirely reasonable, if I'm interpreting it correctly:

Great, I'll make the spec changes and submit them for review here.

> I assume you mean "... the same as it always was" rather than "the same as the
> outgoing case"?

Yes.
Comment 3 Jonny Lamb 2012-03-07 12:25:16 UTC
Okay here's a patch to do this.

HTML is here:

http://people.freedesktop.org/~jonny/telepathy-spec-ft_46783/spec/Channel_Type_File_Transfer1.html

I took this opportunity to try and fix up the rest of the interface. It might just be easier to look at the HTML.
Comment 4 Guillaume Desmottes 2012-04-05 03:10:31 UTC
Maybe it would be worth supporting less dummy protocol where the file hash is sent *after* having sent the file and so saving the sender from reading the file twice on disk.

But I have no idea if any protocol supports this. Maybe jingle file transfer?
Comment 5 Simon McVittie 2012-04-16 07:19:22 UTC
Looking at this again in the context of AccountChannelRequest helper API, it seems wrong that we have a (type, hash) pair. We're basically unable to tell clients which hash type can be used!

We might be better off with something more like this:

property ContentMD5 : s, read, request:sometimes, immutable
property ContentSHA1 : s, read, request:sometimes, immutable
property ContentSHA256 : s, read, request:sometimes, immutable
Comment 6 Jonny Lamb 2012-04-17 09:48:37 UTC
(In reply to comment #4)
> Maybe it would be worth supporting less dummy protocol where the file hash is
> sent *after* having sent the file and so saving the sender from reading the
> file twice on disk.
> 
> But I have no idea if any protocol supports this. Maybe jingle file transfer?

You're right about jingle file transfer.

    http://xmpp.org/extensions/xep-0234.html#hash

Given it's obviously nicer to send the hash afterwards so I'll have a think about it for when we implement jingle FT (any day now!)

(In reply to comment #5)
> Looking at this again in the context of AccountChannelRequest helper API, it
> seems wrong that we have a (type, hash) pair. We're basically unable to tell
> clients which hash type can be used!
> 
> We might be better off with something more like this:
> 
> property ContentMD5 : s, read, request:sometimes, immutable
> property ContentSHA1 : s, read, request:sometimes, immutable
> property ContentSHA256 : s, read, request:sometimes, immutable

Why not a:

property AvailableContentHashTypes : au, read, immutable

Am I missing something here?
Comment 7 Guillaume Desmottes 2012-09-14 09:29:54 UTC
Any progress on this? I'd really like to see bug #42909 fixed so best to fix
the spec in 1.0 first.

(In reply to comment #6)
> (In reply to comment #4)
> > Maybe it would be worth supporting less dummy protocol where the file hash is
> > sent *after* having sent the file and so saving the sender from reading the
> > file twice on disk.
> > 
> > But I have no idea if any protocol supports this. Maybe jingle file transfer?
> 
> You're right about jingle file transfer.
> 
>     http://xmpp.org/extensions/xep-0234.html#hash
> 
> Given it's obviously nicer to send the hash afterwards so I'll have a think
> about it for when we implement jingle FT (any day now!)

We can't know before requesting the channel if the CM is going to use a FT
method allowing the hash to be set afterwards or not (the receiver may or may
not support jingle FT). So we need a new immutable property returned by
CreateChannel telling us this. Maybe something like this:
  ContentHashSupport : No, BeforeSending, AnyTime

If ContentHashSupport == No then ContentHash(Type) are ro during all the
lifetime of the channel.

If ContentHashSupport == BeforeSending then ContentHash(Type) can be set
until ProvideFile() is called.

Not sure until when ContentHash(Type) can be set if ContentHashSupport == AnyTime.
Actually it's not clear to me from the XEP how the receiver is supposed to
know that a hash will be send and so wait for it before presenting the FT has
'done' to the user.

> (In reply to comment #5)
> > Looking at this again in the context of AccountChannelRequest helper API, it
> > seems wrong that we have a (type, hash) pair. We're basically unable to tell
> > clients which hash type can be used!
> > 
> > We might be better off with something more like this:
> > 
> > property ContentMD5 : s, read, request:sometimes, immutable
> > property ContentSHA1 : s, read, request:sometimes, immutable
> > property ContentSHA256 : s, read, request:sometimes, immutable
> 
> Why not a:
> 
> property AvailableContentHashTypes : au, read, immutable
> 
> Am I missing something here?

Maybe that's something that could be merged with ContentHashSupport : uau ?
Comment 8 Guillaume Desmottes 2012-09-14 09:38:50 UTC
(In reply to comment #6)
> property AvailableContentHashTypes : au, read, immutable

Ideally this property should depend of the contact you want to send a file to.
We could use http://xmpp.org/extensions/xep-0300.html#disco to check which
hashing algo are supported by the peer.
Comment 9 Xavier Claessens 2012-09-14 09:49:18 UTC
Even then, jingle could fail and gabble could fallback to normal FT and then it will need the hash sooner. In which case maybe we need a signal "I-need-the-hash-now" and handler then call ProvideHash().
Comment 10 Xavier Claessens 2012-09-14 10:01:58 UTC
(In reply to comment #9)
> Even then, jingle could fail and gabble could fallback to normal FT and then it
> will need the hash sooner. In which case maybe we need a signal
> "I-need-the-hash-now" and handler then call ProvideHash().

Actually forget that :-)


Setting blocker.
Comment 11 GitLab Migration User 2019-12-03 20:26:02 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/telepathy/telepathy-spec/issues/133.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.