After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 308231 - gnomevfs-ls ftp://a@b.c:pass@ftp.gnome.org causes crash
gnomevfs-ls ftp://a@b.c:pass@ftp.gnome.org causes crash
Status: RESOLVED FIXED
Product: gnome-vfs
Classification: Deprecated
Component: Module: ftp
cvs (head)
Other All
: High critical
: ---
Assigned To: Christian Neumair
gnome-vfs maintainers
Depends on:
Blocks:
 
 
Reported: 2005-06-18 22:42 UTC by Olav Vitters
Modified: 2006-03-20 19:53 UTC
See Also:
GNOME target: ---
GNOME version: 2.11/2.12


Attachments
patch to avoid crash in gnomevfs-ls (624 bytes, patch)
2005-07-14 21:40 UTC, Nelson Benitez
needs-work Details | Review
Fix for this bug (1.21 KB, patch)
2005-11-29 15:50 UTC, Fabio Bonelli
reviewed Details | Review
Proposed patch (2.18 KB, patch)
2006-03-19 17:26 UTC, Christian Neumair
committed Details | Review

Description Olav Vitters 2005-06-18 22:42:43 UTC
Steps to reproduce:
1. Run: gnomevfs-ls ftp://a@b.c:pass@ftp.gnome.org
2. 
3. 


Stack trace:
(gdb) bt
  • #0 IA__g_str_hash
    at gstring.c line 98
  • #1 IA__g_hash_table_lookup
    at ghash.c line 193
  • #2 do_open_directory
    at ftp-method.c line 2365
  • #3 open_from_uri
    at gnome-vfs-directory.c line 97
  • #4 gnome_vfs_directory_open
    at gnome-vfs-directory.c line 129
  • #5 main
    at gnomevfs-ls.c line 87

Other information:
Comment 1 Nelson Benitez 2005-07-14 21:40:14 UTC
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.
Comment 2 Christian Neumair 2005-07-14 22:09:12 UTC
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
Comment 3 Alexander Larsson 2005-07-15 09:34:54 UTC
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.
Comment 4 Nelson Benitez 2005-07-15 12:53:36 UTC
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...
Comment 5 Christian Kellner 2005-07-23 23:32:40 UTC
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!
Comment 6 Fabio Bonelli 2005-11-29 15:50:59 UTC
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?
Comment 7 Christian Neumair 2006-03-19 17:13:36 UTC
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.
Comment 8 Christian Neumair 2006-03-19 17:26:26 UTC
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
Comment 9 Christian Neumair 2006-03-20 19:53:46 UTC
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.