Re: [xml] uri again :)

[Daniel Veillard <veillard redhat com>]

On Wed, Mar 19, 2003 at 02:49:50PM -0800, Lorenzo Viali wrote:
> hi again,
>
> there 3 problems i can explain, examples are attached
> in 'test_patch.tar.gz':
> 'bugs' is the output of the 3 programs associated when
> linked against unpatched library.
> 'patch' is the output of the 3 programs associated
> when linked against patched library.
> 'diff bugs patch' is the difference of those outputs.
> 'why' is the bellow explanation of missbehaviours.
> 'Makefile' is the (implicit) input of the 'make'
> program used to built the 3 programs
> the rest of the files are examples that catch the
> questionable behaviour from uri.c exported functions.

  I tried to go through this, it's still quite confusing...

> > > ./patch_ex_xmlNormalizeURIPath <<
> file uri.c, function xmlNormalizeURIPath
>
> 1.
> line 772
> it is doing an extra cur[1] even if cur[0] = '\0'
> (after cur += 3), so outside the end of the string,
> which is *not* allocated.

  Okay that makes sense. Applied. I also applied all the
tests for uri != NULL after changing them so that the
level of predecedences are cleanly indicated with ( and ).
  
> > > ./patch_ex_xmlSaveUri <<
> file uri.c, function xmlSaveUri
> 1.
> line 275
> this is just a choice, but also (source) optimisation:
> whether to represent '/' as literal or as escaped %2F.

  Why did you decide that escaping it was wrong in that
context ? 
  I applied it anyway but I can't get the reason. I don't
know either why I specially changed that code to escape 
/ in opaque parts, this is probably because someone asked for
it, and I have no justification about what is right, this
is unpleasant ...

> > > ./patch_ex_xmlParseURIServer <<
> file uri.c, function xmlParseURIServer
>
> 1.
> it behaves wrong if the first 3 parts of the server
> address, separated by '.', are numeric, the fourth
> also,
> but is followed by some more characters (alnum) that
> make it
> suitable for <hostname>, not <IPv4address>; the bug is
> that it loses
> some part of the hostname, and moreover, it returns a
> parse error!
> e.g. "ftp://18.29.3.30rg.com";
> it goes until after '.30' and now it stops on 'r',
> thinking what?
> that only a ':' (port) or '/' (path) or '?' (query)
> can follow?
> but look, this is actually a valid <server>!!!

  okay 

> 2.
> e.g. "ftp://xml-org."; resolves it to <reg_name>
> 'xml-org.',
> not the <server> 'xml-org.'

  okay

> 3.
> this it is mentioned in the source code
> that it does not handle '-.' (dash dot) group.
> e.g. "ftp://xml-.org"; resolves to <server> 'xml-.org',
> when this should be a <reg-name>.

  okay

> About the big part of the patch, in xmlParseURIServer,
> it's an optimized, corrected and hacked replacement of
> original IPv4address/hostname discrimination
> algorithm.
> - about 10 lines of code -, no 'goto's, no twice
> setting of uri fields authority and server...

  okay applied, I hope it won't break in real use.

Concerning the way to detail the problem, I would really have prefered 
if you extended testURI.c with a new switch to show the various parts
one the URI is parsed than hacking custom tests which are not reusable.

Daniel


-- 
Daniel Veillard      | Red Hat Network https://rhn.redhat.com/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/