On Tue, 2005-08-23 at 00:22 -0400, Charles Schmidt wrote:
> -
> Yet another music sharing patch for people to play with. This should be
> mostly final (in the sense that maybe this could find its way into cvs
> and be worked on from there, soon?).
Yep; do you have a GNOME CVS account already? If not:
http://developer.gnome.org/doc/policies/accounts/requesting.html
> The DNS-SD/Bonjour/Rendezvous isn't done via gnome-vfs anymore
> (previously the daap patch used gnome-vfs [which used howl] to
> browse/discovery daap hosts). It is done directly via howl now. I've
> abstracted that part of the code out so that a drop in replacement
> (avahi) should be relatively easy.
Ok.
> You'll need libsoup & howl to browse music and publish your own. I'm
> planning on writing in support for avahi
> ( http://www.freedesktop.org/Software/Avahi ) as soon as I get it
> working on my machine (damn dbus). Hopefully between the two of them
> people will be able to set up mdns themselves (or, with avahi, have it
> come with their distribution).
Yeah, the OS has to provide this feature, I don't see how we can.
> As before, you'll still need the patch at
> http://bugzilla.gnome.org/show_bug.cgi?id=312114 against gst-plugins.
Ok...I'm compiling gstreamer 0.9 now to see if I can get this patch into
HEAD, and from there backported to 0.8.
More comments on the patch:
+ headers = g_string_new ("Accept: */*\r\nCache-Control: no-cache\r\nUser-Agent: iTunes/4.6 (Windows; N)\r\nClient-DAAP-Access-Index: 2\r\nClient-DAAP-Version: 3.0\r\n");
Shouldn't this use RB_DAAP_USER_AGENT?
+static const char appleCopyright[] = "Copyright 2003 Apple Computer, Inc.";
I suggest we hex-encode (or something) this, and de-hex it at runtime.
That avoids any possible misinterpretation of it as an actual copyright
claim, as opposed to a byte array.
> --- rhythmbox/rhythmdb/rhythmdb.c 2005-08-17 05:37:42.000000000
> -0400
> +++ myrhythmbox/rhythmdb/rhythmdb.c 2005-08-22 21:40:26.000000000
> -0400
> @@ -1611,6 +1611,18 @@
>
> GNOME_VFS_FILE_INFO_FOLLOW_LINKS))
> == GNOME_VFS_OK)
> return;
> + if (g_str_has_prefix (uri, "daap://")) {
> + GnomeVFSFileInfo *info = event->vfsinfo;
> +
> + info->name = g_strdup(strrchr(uri,'/') + 1);
> + info->valid_fields =
> GNOME_VFS_FILE_INFO_FIELDS_PERMISSIONS |
> GNOME_VFS_FILE_INFO_FIELDS_TYPE | GNOME_VFS_FILE_INFO_FIELDS_FLAGS;
> +
> + info->type = GNOME_VFS_FILE_TYPE_REGULAR;
> + info->flags = GNOME_VFS_FILE_FLAGS_NONE;
> + info->permissions = GNOME_VFS_PERM_USER_READ;
> +
> + return;
> + }
> error:
Is this still necessary? I fixed a bug a while back in the core that
statted all entries, not just songs.
> if (buf_length < codesize + 8) {
> + g_warning ("Malformed response");
> + return;
> + }
Two things here.
First, I'd prefer if we didn't g_warning for this. Potentially it will
happen due to malicious input, better we don't fill up the user's
~/.xsession-log or whatever; attacks should just silently fail.
Second, the "+ 8" there has a few problems. Won't it potentially cause
us to lose the last daap item? Or does the DAAP protocol have a padding
of 8? Also, this is vulnerable to an integer overflow; if the attacker
specifies G_MAXINT, + 8 will overflow to a negative value, passing your
check. Also, when the check returns, we leak item and node.
I know it's a pain, but getting this stuff right is very important.
> Issues/Known Bugs:
> * Signed errors when compiling. According to walters (i think), gcc 4.0
> added the signed checking thing. I havn't found time to find packages
> for 4.0 for my system yet, or I'd try to fix this myself. Until then, I
> suggest writing me a patch or turning off the extra warning checking.
I think James had a patch.
> * The patch from http://bugzilla.gnome.org/show_bug.cgi?id=167364
> "broke" seeking for daap streams. You can still seek in them just fine,
> but because the GStreamer playbin isn't aware that you have, it can't
> report the time properly. While I agree that the "new" way
> (gst_element_query) is better, I can't see any way to get the time
> reported for daap streams except to do it the "old" way. Any thoughts?
Hmmm.
> * There isn't really any error reporting yet.
> * When sharing your music, if the share name already exists on the
> network, Rhythmbox won't notify you of this.
Hopefully unlikely...Also, aren't we automatically renaming in the case
of conflict?
> * When loading a daap server the user interface is not updated until it
> the network communications are finished. Ideally, the network stuff
> should be in a separate thread, or at the very least the daap buffer
> parsing stuff should be done incrementally in a timeout.
Yeah...this is going to be important to fix soon actually; consider an
office where you have say 50 people sharing music...
> * No authentication.
Not high priority in my opinion.
> * Anywhere else there's a 'FIXME'
>
> So. Get out there and try it, please!!
Attachment:
signature.asc
Description: This is a digitally signed message part