Re: [PATCH] Fix heavily broken relative URI resolution

[Christian Neumair <chris gnome-de org>]

Am Donnerstag, den 20.04.2006, 16:03 +0200 schrieb Alexander Larsson:
> On Sun, 2006-03-19 at 17:06 +0100, Christian Neumair wrote:
> > Am Sonntag, den 19.03.2006, 13:09 +0100 schrieb Christian Neumair:
> > > The attached patch implements the relative URI resolution as suggested
> > > by RFC 2396 and RFC 3986. The old algorithm was quite hosed, for
> > > instance when the relative reference had too many ".." components, which
> > > overwrote the authority component of an URI.
> > > "ftp://server/foo";, "../../../" would be resolved to some garbage like
> > > "ftp:///";.
> > > 
> > > After applying this patch, test-uri passes all URI relative resolution
> > > tests in RFC 3986.
> 
> I'm a bit worried by this patch. It rewrites some pretty core parts of
> gnome-vfs, and it mentions RFCs a lot.

Yes, because resolve_relative claims to conform to an RFC, where it
doesn't, and is used way too often, for instance for resolving a
relative path name or file name. It's simply a helper that does text
manipulation according to an RFC, and the docs produce the impression
that it can be used for things it wasn't meant to be used for. For
example, _gnome_vfs_uri_resolve_all_symlinks_uri should use
gnome_vfs_resolve_symlink instead of resolve_relative. The ftp method
also makes use of it.

> gnome-vfs uris are not really the
> same as web uris, for instance # is used for uri-chaining, not
> fragments, so things like:
> 
> if (*scheme == NULL) /* only relative references may have fragments! */
> 	f = strchr (p, '#');
> 
> look wrong. For instance ftp://ftp.gnome.org/pub/baa.zip#zip:///README
> is a valid gnome-vfs uri.
> 
> I worry that there are other gnome-vfs specific details in uri handling
> that can accidentally get broken with this change...
> 
> Does this fix any important issues in practice, or is it more of a
> cleanness fix?

It doesn't fix nor further break any issues, because I'm not aware of
any client application using the _resolve_relative helper correctly,
i.e. for what it was meant to be used. People who rely on standard
compliant software will still find it helpful, though - although these
persons may not be interested in GnomeVFSURIs at all, so we should
provide a text-only variant (gnome_vfs_resolve_relative).

> > > I'm not sure why the old code was laid out that way, but somebody
> > > probably didn't separate relative URI resolution and symblic link
> > > resolution, which are two completely different things. The latter is for
> > > instance implemented in my sftp method patch, and could be used by other
> > > methods as well. One notable difference is that trailing slashes will be
> > > preserved in the RFC's algorithm. I'm not sure how desireable this is
> > > for directories, for instance /home/foo where foo is a symlink to "..",
> > > this would be resolved to /home/ with the RFC algorithm, and to /home
> > > with the symlink algorithm, I'm not sure who should handle the
> > > canonicalization, i.e. ensure that get_file_info for /home/ will work
> > > if /home is a symlink.
> > > 
> > > There is quite a similarity between _gnome_vfs_canonicalize_pathname and
> > > remove_dot_segments, however I couldn't figure out the details because I
> > > don't find _gnome_vfs_canonicalize_pathname particularly traceable.
> > > 
> > > I don't know WHY many modules and applications use
> > > gnome_vfs_uri_resolve_relative, they usually shouldn't have to mess
> > > around with this and instead use a symlink resolution helper - maybe
> > > because the old docs claim it has something to do with relative path
> > > resolution, which is only right to a point where the relative URI only
> > > forms a path with segments that exclusively contain unreserved
> > > characters.
> 
> Yeah, we should be using a specific symlink resolver in some places
> instead of resolve_relative.

-- 
Christian Neumair <chris gnome-de org>