8.0-RC USB/FS problem

[Jeremy Chadwick]

On Tue, Nov 24, 2009 at 12:16:54PM -0600, Dan Nelson wrote:
> In the last episode (Nov 24), Jeremy Chadwick said:
> > On Tue, Nov 24, 2009 at 06:13:21PM +0100, Hans Petter Selasky wrote:
> > > On Tuesday 24 November 2009 17:58:47 Guojun Jin wrote:
> > > > Sorry for the typo -- it is public not pub in the middle. The others should
> > > > be all public.
> > > >
> > > > http:/www.daemonfun.com/archives/public/USB/crash1-reset.bz2
> > > >
> > > 
> > > %fetch http:/www.daemonfun.com/archives/public/USB/crash1-reset.bz2
> > > fetch: http:/www.daemonfun.com/archives/public/USB/crash1-reset.bz2: No address record
> > 
> > The above issue is unrelated to the USB/FS problem.  It looks like
> > fetch(1) has a parser bug.  Note the text portion between the URI and URL
> > is colon-slash not colon-slash-slash like it should be.
> 
> That's a typo in the URL, not a bug in fetch :)

It's a bug in libfetch, specifically the fetchParseURL(3) function.
Relevant code from src/usr.bin/fetch/fetch.c:

 312 static int
 313 fetch(char *URL, const char *path)
 314 {
 315         struct url *url;
 ...
 342         /* parse URL */
 343         if ((url = fetchParseURL(URL)) == NULL) {
 344                 warnx("%s: parse error", URL);
 345                 goto failure;
 346         }

The man page for fetchParseURL(3) claims:

     fetchParseURL() takes a URL in the form of a null-terminated string and
     splits it into its components function according to the Common Internet
     Scheme Syntax detailed in RFC1738.  A regular expression which produces
     this syntax is:

         <scheme>:(//(<user>(:<pwd>)?@)?<host>(:<port>)?)?/(<document>)?

     If the URL does not seem to begin with a scheme name, the following syn-
     tax is assumed:

         ((<user>(:<pwd>)?@)?<host>(:<port>)?)?/(<document>)?

     .....

     fetchParseURL() returns a pointer to a struct url containing the individ-
     ual components of the URL.  If it is unable to allocate memory, or the
     URL is syntactically incorrect, fetchParseURL() returns a NULL pointer.

If we add some debugging code *before* the "scheme assumption" portion
of fetch.c (which actually looks at the hostname portion and if it
starts with "ftp" assumes the scheme is FTP, "http" = HTTP, etc.):

 348         printf("fetchParseURL() successful.  struct details:\n");
 349         printf("url->scheme = %s\n", url->scheme);
 350         printf("url->user   = %s\n", url->user);
 351         printf("url->pwd    = %s\n", url->pwd);
 352         printf("url->host   = %s\n", url->host);
 353         printf("url->port   = %d\n", url->port);
 354         printf("url->doc    = %s\n", url->doc);

...we end up with this:

$ ./fetch http:/www.daemonfun.com/
fetchParseURL() successful.  struct details:
url->scheme = http
url->user   =
url->pwd    =
url->host   =
url->port   = 0
url->doc    = /www.daemonfun.com/
fetch: http:/www.daemonfun.com/: No address record

Here we can see the libfetch code properly works out the scheme (URI) on
its own (which means the "assumption part" of the man page should not
play a role here) -- but incorrectly parses the remaining portion of the
URL.

In this situation, fetchParseURL(3) should return NULL.

The code in fetch.c continues on to call fetchXGet(3) with the above
struct data (some of it gets modified, but that's besides the point),
and the result is fetchXGet(3) returning NULL (indicating the fetch
failed in some way), which gets us here:

 463         f = fetchXGet(url, &us, flags);
 ...
 468         if (f == NULL) {
 469                 warnx("%s: %s", URL, fetchLastErrString);
 470                 if (i_flag && strcmp(url->scheme, SCHEME_HTTP) == 0
 471                     && fetchLastErrCode == FETCH_OK
 472                     && strcmp(fetchLastErrString, "Not Modified") == 0) {
 473                         /* HTTP Not Modified Response, return OK. */
 474                         r = 0;
 475                         goto done;
 476                 } else
 477                         goto failure;
 478         }

We modify some code to add some debugging to validate that warnx() is
what's returning the error message:

 469                 warnx("fetchXGet() returned NULL: %s: %s", URL, fetchLastErrString);

...and we end up with:

fetch: fetchXGet() returned NULL: http:/www.daemonfun.com/: No address record

I guess I'll be the one to file the PR.

-- 
| Jeremy Chadwick                                   jdc at parodius.com |
| Parodius Networking                       http://www.parodius.com/ |
| UNIX Systems Administrator                  Mountain View, CA, USA |
| Making life hard for others since 1977.              PGP: 4BD6C0CB |