GNOME Bugzilla – Bug 308231
gnomevfs-ls ftp://a@b.c:pass@ftp.gnome.org causes crash
Last modified: 2006-03-20 19:53:46 UTC
Steps to reproduce: 1. Run: gnomevfs-ls ftp://a@b.c:pass@ftp.gnome.org 2. 3. Stack trace: (gdb) bt
+ Trace 61235
Other information:
Created attachment 49189 [details] [review] patch to avoid crash in gnomevfs-ls I can confirm this bug. Anybody can test this crash and the patch I provide with the following ftp account I've created on my server (here you see output after patch applied): $ gnomevfs-ls ftp://testgnome@cenobioracing.com:testgnome@ftp.cenobioracing.com Error opening: Host name not valid $ Also note that to pass a @ in the username or password field of an uri it has to be escaped, this way "ftp://a%40b.c:pass@ftp.gnome.org", of course the application using gnome-vfs should do the escaping for you, but still gnome-vfs fails with a legal escaped uri like that, I putted a patch to fix that in #89106.
Excellent work! Thanks for your efforts. Maybe you could submit these two patches to gnome-vfs-list [1] as well for review? :) [1] http://mail.gnome.org/mailman/listinfo/gnome-vfs-list
This doesn't look quite right. You're just avoiding to crash by not looking things up in the cache. But then the NULL text->uri is added to the cache, even though it will not later be used due to being ignored by your patch.
Looking at the patch I dont see your point, I am just avoiding to crash by not looking things up in the cache *when text->uri is NULL* , and when that is the case, text->uri is *not* added to the cache because cached_dirlist is still NULL (it cannot have other value because we didn't look up in the cache)... so I dont see why you say "NULL text->uri is added to the cache", please precise on this...
Well I get Alex' point here. Assuming text->uri will be NULL, do_path_command will do a "CWD /" in that case (this function can handle path == NULL), the LIST command is likely to succeed and then we have a g_hash_table_replace () at line 2446 of ftp-method.c with text->uri == NULL. We should maybe special case the text->uri == NULL case from the very beginning. I might look into that tomorrow in detail. Thanks for your efforts though!
Created attachment 55375 [details] [review] Fix for this bug This patch fixes the crash. BTW: shouldn't libgnomevfs always pass a "good" uri to a module?
Fabio: Your patch looks very unrelated but filtering out NULL-hostnames seems to be a good idea. You should guard all method functions with it, though. > BTW: shouldn't libgnomevfs always pass a "good" uri to a module? I've read the URI RFCs during the two days, and they claim that a host (i.e. authority-less) URI, even with an empty path, i.e. "foo:" is perfectly OK. It's clearly the module's task to filter out URIs that are considered bad or not meaningful in a particular context.
Created attachment 61557 [details] [review] Proposed patch This one obsoletes Nelson's patch. Discussion: http://mail.gnome.org/archives/gnome-vfs-list/2006-March/msg00029.html
Attachment 61557 [details] was committed, closing. Fabio: It would be nice to have patches against all network methods, adding if (gnome_vfs_uri_get_host_name (uri) == NULL) return GNOME_VFS_ERROR_INVALID_HOST_NAME; protectors in front of all do_foo implementations.