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
Received on 2016-11-11