Re: [xml] relative uri

[Daniel Veillard <veillard redhat com>]

On Tue, Oct 20, 2009 at 04:51:58PM +0200, François Delyon wrote:
> Hi all,
>
> I have some remarks about the implementation of xmlBuildRelativeURI  in 
> uri.c (libxml 2.7.3)
>
> 1-critical bug
> line 2371
>       if ((ref->path[pos] == '.') && (ref->path[pos+1] == '/'))
>             pos += 2;
>       if ((*bptr == '.') && (bptr[1] == '/'))
>             bptr += 2;
>       else if ((*bptr == '/') && (ref->path[pos] != '/'))
>           bptr++;
>       while ((bptr[pos] == ref->path[pos]) && (bptr[pos] != 0))
>           pos++;
>
>       if (bptr[pos] == ref->path[pos]) {
>           val = xmlStrdup(BAD_CAST "");
>           goto done;          /* (I can't imagine why anyone would do this) */
>       }
>
> the use of bptr[pos]  is irrelevant. I suspect that the first four lines 
> have been added quickly to solve some bad case.

 Why ? Example, reason to your jugement ?

> My opinion is that the four first lines are not very important (the same 
> kind of problem may occur later in the path) and that  

 that's basically looks like URI normalization see 
   http://tools.ietf.org/html/rfc3986
section 5.2.4, though that code was done with preceeding RFC 2396 in
mind.

> xmlBuildRelativeURI should normalize the paths before processing in  
> order to get human and efficient  relative uris.

  yes that could be improved that way, using xmlNormalizeURIPath()

> 2- bad answer.
> if the basepath is "/a/b" and the path is "/a/c:d", xmlBuildRelativeURI 
> return "c:d" which is not a relative uri. The relative uri is "./c:d"
> This remark is perhaps not very useful for libxml2 since  
> xmlBuildRelativeURI is called in very few places.

  Hum, right that's a special case if : is in the first segment of the
remaining path, but it sounds more like an escaping problem to me, the :
should be escaped to make sure it's not misinterpreted at parsing time
  "c%3Ad" rather than "./c:d"
one way is to extend the xmlURIEscapeStr at the end to also convert :

> 3- cosmetic problem
> line 2392
>       else if ((ref->path[ix] == 0) && (ix > 1) && (ref->path[ix - 1] ==  
> '/'))
>           ix -= 2;
> basepath "/a/b" path "/a/c" gives "../a/c" where I expect "c"
> and
> basepath "/a/b" path "/a/" gives "../a/" where I expect "."

  hum, more fixes and tests would be nice, yes

> 4-question
>  xmlBuildRelativeURI  return things like xmlURIEscapeStr(uptr, BAD_CAST 
> "/;&=+$,").
> Must I understand that the path in a xmlURIPtr is not escaped?
> I think that I missed something.

  the strings in xmlURI, i.e. after parsing are no more escaped.
but since we return a serialized URI then escaping need to be done
there.

> ------
>
> I am not familiar with the development of libxml, but I can suggest the 
> following code to build a relative path.

  Can you please apply some of the above comments and provide a
contextual diff, (using diff -c or dff -u on the files before or after
the change, with git, a git diff on the command line will show you the
diff from the modified files) . Then provide the patch as an attachment
to make sure it won't me modified by mail transmission,

 thanks,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/