Bug 23160 - add AIM's strange hash algorithm and whatever else is needed for FT
Summary: add AIM's strange hash algorithm and whatever else is needed for FT
Status: RESOLVED MOVED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-spec (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 24919 24920 24921
  Show dependency treegraph
 
Reported: 2009-08-05 10:14 UTC by Dafydd Harries
Modified: 2019-12-03 20:18 UTC (History)
5 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Dafydd Harries 2009-08-05 10:14:34 UTC
Some protocols allow or require a checksum to be sent at the beginning of a file transfer. Currently, we require the client initiating the file transfer to calculate file transfer checksums. Problems with this:

 1. AIM uses an obscure hash algorishm (and requires the checksum)

Solutions:

 0. Declare that requiring clients to implement checksums is not onerous, and maintain the status quo.

 1. Implement checksums in the CM. Have the CM be able to read the file twice by:

   a) Have clients send the file to the CM before making the offer, and make the CM buffer the file in memory until the offer is accepted or rejected.

   b) Have clients send the file to the CM before making the offer, and a second time when the offer is accepted.

   c) Allowing clients to pass a filename to the CM, and having the CM read the file.

   d) Waiting for file descriptor passing to be included in D-Bus, and allowing clients to pass an open seekable file descriptor to the connection manager before sending the file transfer offer.

My preferred approaches are 0 and 1d.
Comment 1 Olivier Crête 2009-08-05 10:44:02 UTC
0 is nasty because we need every client implementation to implement every checksum known to man..

I propose changing to spec to separate the "provide me the file" from being "when the transfer is "open"" to having a special signal for that "PleaseProvideFile" or something.

Then use the current API.. ie give a fifo to the CM.. This has the advantage of being full compatible with the current API. In the AIM case, we can just emit PleaseProvideFile twice.

And then later when dbus 1.4 is upon us, we can add another API (ProvideFD) to provide the FD to the CM directly with a flag saying if its seekable or not. If its not seekable, then the CM can decide to re-emit the signal and get a new FD or something. Even in the non-hash case, that will reduce the number of context switches and should also improve performance.
Comment 2 Dafydd Harries 2009-08-05 10:51:26 UTC
(In reply to comment #1)
> 0 is nasty because we need every client implementation to implement every
> checksum known to man..

I'm not convinced it's actually that bad. We only know of one obscure algorithm in practice.

> I propose changing to spec to separate the "provide me the file" from being
> "when the transfer is "open"" to having a special signal for that
> "PleaseProvideFile" or something.
> 
> Then use the current API.. ie give a fifo to the CM.. This has the advantage of
> being full compatible with the current API. In the AIM case, we can just emit
> PleaseProvideFile twice.
> 
> And then later when dbus 1.4 is upon us, we can add another API (ProvideFD) to
> provide the FD to the CM directly with a flag saying if its seekable or not. If
> its not seekable, then the CM can decide to re-emit the signal and get a new FD
> or something. Even in the non-hash case, that will reduce the number of context
> switches and should also improve performance.

Well, the CM doesn't need to be told whether it's seekable; it can try to seek and ask for another one if it's not.

Comment 3 Simon McVittie 2009-08-05 10:52:55 UTC
Note that FileTransfer has been undrafted (and is part of telepathy-glib's ABI), so incompatible API changes should be avoided, and we should continue to support all current FT clients (at least for the file transfers they can currently do - XMPP and local-XMPP).

As such, FT clients will have to do *some* extra work to support AIM file transfers, for any of [0], [1a], [1b], [1c], [1d].

Like Daf said, I don't actually think [0] is such a big deal: I don't expect the list of checksums to get longer than 5-6 (SHA1, SHA256 and MD5 we have, strange-AIM-checksum we need to add, and maybe CRC32 and/or Adler32 for some miscellaneous protocol or other). It wouldn't be rocket science to have support for all of these in each of our current bindings (GLib, Qt4 and Python).
Comment 4 Olivier Crête 2009-08-05 11:34:27 UTC
@daf: Are you volunteering to implement it in C, C++, javascript, elisp, python, perl, misc, others?
Comment 5 Jonny Lamb 2009-08-07 01:46:34 UTC
(In reply to comment #1)
> 0 is nasty because we need every client implementation to implement every
> checksum known to man..

That's great, but unless you know of protocols hiding away that require misc other hashes, I think in practice this isn't such a big deal besides the annoying AIM algorithm.

(In reply to comment #3)
> It wouldn't be rocket science to have support for all of these in each of
> our current bindings (GLib, Qt4 and Python).

Right. This is what I've been proposing all along. In reality, every client uses one of the three bindings. If they don't, then they've probably got bigger problems.

Of course, option 0 is also the nicest in my book because it requires no changes to existing clients (besides implementing this said hash). gabble and salut will not need to change, and with a stable GNOME release coming up, this is useful, perhaps.
Comment 6 Simon McVittie 2009-11-04 10:21:24 UTC
(In reply to comment #3)
> Like Daf said, I don't actually think [0] is such a big deal: I don't expect
> the list of checksums to get longer than 5-6 (SHA1, SHA256 and MD5 we have,
> strange-AIM-checksum we need to add, and maybe CRC32 and/or Adler32 for some
> miscellaneous protocol or other).

The first step is to gather the required hashing algorithms (for completeness, maybe do everything that's mentioned in libpurple?) and put them in telepathy-spec.
Comment 7 GitLab Migration User 2019-12-03 20:18:48 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/34.


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.