On Friday, November 11, 2016 11:40:25 Daniel Stenberg wrote:
> On Fri, 11 Nov 2016, Kamil Dudka wrote:
> > I would prefer to get a more descriptive error message saying what exactly
> > was matched as the host part and what was expected there for the URL to be
> > accepted.
> 
> Good point. But since the code hasn't exactly extracted the found hostname
> correctly, it's not that easy to show it. How about at least making it say:
> 
>    failf(data, "Valid host name with slash missing in URL");

Thanks for the update!

> The funny phrasing because it actually checks for "localhost/" so the error
> string will also be shown for "file://localhost": a file: URL without a
> trailing slash.

Yes, we do not expect a "valid host name".  What about the following wording?

    "Expected 'localhost' or empty string as host name in file:// URL"

Note that I would not bother with the 127.0.0.1 variant in the error message 
because we accept it only for compatibility reasons as I understand it.

> > One minor nit.  Can we write:
> >    if ('/' == ptr[1])
> > 
> > ... instead of:
> >    if(ptr[1] && ('/' == ptr[1]))
> 
> It actually has to be changed to
> 
>    if(ptr[0] && ('/' == ptr[1]))

Now I am confused.  Which case should it actually cover?

By reverse engineering I made up a URL "file://localhost///etc/fstab" to get 
there ... but it does not look like a case we need to handle specially :-)
So what is the purpose of this condition?  Which URLs do we need it for?

Kamil

> So that it doesn't read beyond the string for "file://localhost/".
> 
> Thanks a lot for the comments. I've attached my updated version.
-------------------------------------------------------------------
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:  https://curl.haxx.se/mail/etiquette.html

Reply via email to