NetBSD Problem Report #43013
From www@NetBSD.org Fri Mar 19 14:47:13 2010
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
by www.NetBSD.org (Postfix) with ESMTP id B6CD863B86C
for <gnats-bugs@gnats.NetBSD.org>; Fri, 19 Mar 2010 14:47:13 +0000 (UTC)
Message-Id: <20100319144713.4D54263B11D@www.NetBSD.org>
Date: Fri, 19 Mar 2010 14:47:13 +0000 (UTC)
From: brook@nmsu.edu
Reply-To: brook@nmsu.edu
To: gnats-bugs@NetBSD.org
Subject: Clarification of URL handling for pkg_add/pkgin
X-Send-Pr-Version: www-1.0
>Number: 43013
>Category: pkg
>Synopsis: Clarification of URL handling for pkg_add/pkgin
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: joerg
>State: closed
>Class: doc-bug
>Submitter-Id: net
>Arrival-Date: Fri Mar 19 14:50:00 +0000 2010
>Closed-Date: Sat Nov 20 23:38:35 +0000 2010
>Last-Modified: Sat Nov 20 23:38:35 +0000 2010
>Originator: Brook Milligan
>Release: 5.0/i386
>Organization:
New Mexico State University
>Environment:
NetBSD viola.nmsu.edu 5.0_STABLE NetBSD 5.0_STABLE (VIOLA) #0: Thu Jan 28 19:18:43 MST 2010 root@viola.nmsu.edu:/usr/obj/sys/arch/i386/compile/VIOLA i386
>Description:
I recently tried to use pkg_add and pkgin with a remote repository for
the first time. Being very familiar with using full URLs with ftp, I
interpreted the man pages for setting up these programs to mean that
the URLs specified to them would be equivalent to those specified to
ftp for downloading the same file. This of course did not work
because of the following:
- ftp interprets paths relative to the default login directory.
- pkg_add/pkgin use libfetch which precedes accesses with CDUP
commands to reach the root; therefore, paths are relative to the
root of the ftp server.
In my case, these were not the same place, so while ftp worked fine
the other commands did not. Needless to say, this was confusing.
I suggest that an addition to the pkg_add man page (see below) might
have helped me avoid some confusion.
Please improve this if it is technically incorrect or incomplete
(e.g., if http behaves differently), but this does appear to be the
behavior I have observed with ftp.
>How-To-Repeat:
Create an ftp repository with a login directory that is different from the root. Retreive a file with ftp using a URL relative to the login directory. Try to use pkg_add or pkgin to retrieve the same file with the same URL. It will fail. Successfully retrieve the file with pkg_add or pkgin using a URL relative to the root.
>Fix:
Note that this patches only pkg_add.1; it seems that pkg_add.cat1 also exists in the distribution and should be made consistent.
--- pkg_add.1.orig 2010-03-19 08:41:02.000000000 -0600
+++ pkg_add.1 2010-03-19 08:42:22.000000000 -0600
@@ -212,6 +212,8 @@
.Dq .tgz
suffix) or a
URL pointing at a file available on an ftp or web site.
+In the latter case, URLs will be resolved relative to the root of the
+server filesystem not the login directory.
Thus you may extract files directly from their anonymous ftp or WWW
locations (e.g.,
.Nm
>Release-Note:
>Audit-Trail:
Responsible-Changed-From-To: pkg-manager->joerg
Responsible-Changed-By: joerg@NetBSD.org
Responsible-Changed-When: Fri, 19 Mar 2010 18:06:45 +0000
Responsible-Changed-Why:
RFC1738 bug in fetch(3)
From: Joerg Sonnenberger <joerg@britannica.bec.de>
To: gnats-bugs@NetBSD.org
Cc: brook@nmsu.edu
Subject: Re: pkg/43013: Clarification of URL handling for pkg_add/pkgin
Date: Fri, 19 Mar 2010 19:08:07 +0100
--bp/iNruPH9dso1Pn
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Attached is a patch to correctly implement the desired RFC1738 behavior
for FTP.
Joerg
--bp/iNruPH9dso1Pn
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="ftp-urls.diff"
Index: common.c
===================================================================
RCS file: /home/joerg/repo/netbsd/src/external/bsd/fetch/dist/libfetch/common.c,v
retrieving revision 1.1.1.8
diff -u -p -r1.1.1.8 common.c
--- common.c 30 Jan 2010 21:26:09 -0000 1.1.1.8
+++ common.c 19 Mar 2010 14:25:55 -0000
@@ -231,6 +231,7 @@ fetch_reopen(int sd)
/* allocate and fill connection structure */
if ((conn = calloc(1, sizeof(*conn))) == NULL)
return (NULL);
+ conn->ftp_home = NULL;
conn->cache_url = NULL;
conn->next_buf = NULL;
conn->next_len = 0;
@@ -711,6 +712,7 @@ fetch_close(conn_t *conn)
ret = close(conn->sd);
if (conn->cache_url)
fetchFreeURL(conn->cache_url);
+ free(conn->ftp_home);
free(conn->buf);
free(conn);
return (ret);
Index: common.h
===================================================================
RCS file: /home/joerg/repo/netbsd/src/external/bsd/fetch/dist/libfetch/common.h,v
retrieving revision 1.1.1.6
diff -u -p -r1.1.1.6 common.h
--- common.h 30 Jan 2010 21:26:09 -0000 1.1.1.6
+++ common.h 19 Mar 2010 14:24:51 -0000
@@ -73,6 +73,8 @@ struct fetchconn {
# endif
#endif
+ char *ftp_home;
+
struct url *cache_url;
int cache_af;
int (*cache_close)(conn_t *);
Index: ftp.c
===================================================================
RCS file: /home/joerg/repo/netbsd/src/external/bsd/fetch/dist/libfetch/ftp.c,v
retrieving revision 1.1.1.10
diff -u -p -r1.1.1.10 ftp.c
--- ftp.c 30 Jan 2010 21:26:13 -0000 1.1.1.10
+++ ftp.c 19 Mar 2010 15:56:05 -0000
@@ -252,7 +252,7 @@ ftp_filename(const char *file, int *len,
* command.
*/
static int
-ftp_pwd(conn_t *conn, char *pwd, size_t pwdlen)
+ftp_pwd(conn_t *conn, char **pwd)
{
char *src, *dst, *end;
int q;
@@ -264,7 +264,10 @@ ftp_pwd(conn_t *conn, char *pwd, size_t
src = conn->buf + 4;
if (src >= end || *src++ != '"')
return (FTP_PROTOCOL_ERROR);
- for (q = 0, dst = pwd; src < end && pwdlen--; ++src) {
+ *pwd = malloc(end - src + 1);
+ if (*pwd == NULL)
+ return (FTP_PROTOCOL_ERROR);
+ for (q = 0, dst = *pwd; src < end; ++src) {
if (!q && *src == '"')
q = 1;
else if (q && *src != '"')
@@ -274,9 +277,12 @@ ftp_pwd(conn_t *conn, char *pwd, size_t
else
*dst++ = *src;
}
- if (!pwdlen)
- return (FTP_PROTOCOL_ERROR);
*dst = '\0';
+ if (**pwd != '/') {
+ free(*pwd);
+ *pwd = NULL;
+ return (FTP_PROTOCOL_ERROR);
+ }
return (FTP_OK);
}
@@ -285,69 +291,108 @@ ftp_pwd(conn_t *conn, char *pwd, size_t
* file.
*/
static int
-ftp_cwd(conn_t *conn, const char *file, int subdir)
+ftp_cwd(conn_t *conn, const char *path, int subdir)
{
const char *beg, *end;
- char pwd[PATH_MAX];
+ char *pwd, *dst;
int e, i, len;
- /* If no slashes in name, no need to change dirs. */
- if (subdir)
- end = file + strlen(file);
- else if ((end = strrchr(file, '/')) == NULL)
- return (0);
+ if (*path != '/') {
+ ftp_seterr(501);
+ return (-1);
+ }
+ ++path;
+
+ /* Simple case: still in the home directory and no directory change. */
+ if (conn->ftp_home == NULL && strchr(path, '/') == NULL)
+ return 0;
+
if ((e = ftp_cmd(conn, "PWD\r\n")) != FTP_WORKING_DIRECTORY ||
- (e = ftp_pwd(conn, pwd, sizeof(pwd))) != FTP_OK) {
+ (e = ftp_pwd(conn, &pwd)) != FTP_OK) {
ftp_seterr(e);
return (-1);
}
+ if (conn->ftp_home == NULL && (conn->ftp_home = strdup(pwd)) == NULL) {
+ fetch_syserr();
+ free(pwd);
+ return (-1);
+ }
+ if (*path == '/') {
+ while (path[1] == '/')
+ ++path;
+ dst = strdup(path);
+ } else if (strcmp(conn->ftp_home, "/") == 0) {
+ dst = strdup(path - 1);
+ } else {
+ asprintf(&dst, "%s/%s", conn->ftp_home, path);
+ }
+ if (dst == NULL) {
+ fetch_syserr();
+ free(pwd);
+ return (-1);
+ }
+
+ if (subdir)
+ end = dst + strlen(dst);
+ else
+ end = strrchr(dst, '/');
+
for (;;) {
len = strlen(pwd);
/* Look for a common prefix between PWD and dir to fetch. */
- for (i = 0; i <= len && i <= end - file; ++i)
- if (pwd[i] != file[i])
+ for (i = 0; i <= len && i <= end - dst; ++i)
+ if (pwd[i] != dst[i])
break;
/* Keep going up a dir until we have a matching prefix. */
if (strcmp(pwd, "/") == 0)
break;
- if (pwd[i] == '\0' && (file[i - 1] == '/' || file[i] == '/'))
+ if (pwd[i] == '\0' && (dst[i - 1] == '/' || dst[i] == '/'))
break;
+ free(pwd);
if ((e = ftp_cmd(conn, "CDUP\r\n")) != FTP_FILE_ACTION_OK ||
(e = ftp_cmd(conn, "PWD\r\n")) != FTP_WORKING_DIRECTORY ||
- (e = ftp_pwd(conn, pwd, sizeof(pwd))) != FTP_OK) {
+ (e = ftp_pwd(conn, &pwd)) != FTP_OK) {
ftp_seterr(e);
+ free(dst);
return (-1);
}
}
+ free(pwd);
#ifdef FTP_COMBINE_CWDS
/* Skip leading slashes, even "////". */
- for (beg = file + i; beg < end && *beg == '/'; ++beg, ++i)
+ for (beg = dst + i; beg < end && *beg == '/'; ++beg, ++i)
/* nothing */ ;
/* If there is no trailing dir, we're already there. */
- if (beg >= end)
+ if (beg >= end) {
+ free(dst);
return (0);
+ }
/* Change to the directory all in one chunk (e.g., foo/bar/baz). */
e = ftp_cmd(conn, "CWD %.*s\r\n", (int)(end - beg), beg);
- if (e == FTP_FILE_ACTION_OK)
+ if (e == FTP_FILE_ACTION_OK) {
+ free(dst);
return (0);
+ }
#endif /* FTP_COMBINE_CWDS */
/* That didn't work so go back to legacy behavior (multiple CWDs). */
- for (beg = file + i; beg < end; beg = file + i + 1) {
+ for (beg = dst + i; beg < end; beg = dst + i + 1) {
while (*beg == '/')
++beg, ++i;
- for (++i; file + i < end && file[i] != '/'; ++i)
+ for (++i; dst + i < end && dst[i] != '/'; ++i)
/* nothing */ ;
- e = ftp_cmd(conn, "CWD %.*s\r\n", file + i - beg, beg);
+ e = ftp_cmd(conn, "CWD %.*s\r\n", dst + i - beg, beg);
if (e != FTP_FILE_ACTION_OK) {
+ free(dst);
ftp_seterr(e);
return (-1);
}
}
+ free(dst);
return (0);
}
--bp/iNruPH9dso1Pn--
From: brook@biology.nmsu.edu (Brook Milligan)
To: gnats-bugs@NetBSD.org
Cc: joerg@NetBSD.org, gnats-admin@netbsd.org, pkgsrc-bugs@netbsd.org,
brook@nmsu.edu
Subject: Re: pkg/43013: Clarification of URL handling for pkg_add/pkgin
Date: Fri, 19 Mar 2010 22:14:30 -0600
Joerg,
I applied your patch, but the same problem remains. As a result, I've
been looking over the code some more and have a few points to note.
I think I have narrowed down the problem that I am seeing. Here is
one section of the calling sequence as indicated by some printf()
statements I added (note that the URL I gave to pkg_add was
ftp://localhost/All/emacs and that the file All/emacs-23.1nb2.tgz
exists in the ftp login directory):
fetchList(): pattern=emacs*, flags=c
fetchListFTP(): NLST
ftp_request(): op=NLST, op_arg=emacs*, flags=c
ftp_request(): path=/All
ftp_cwd(): path=/All, subdir=1
ftp_cwd(): path=All
ftp_cwd(): conn->ftp_home=(null)
ftp_cwd(): strchr(path, '/')=(null)
I think the key point here is the subdir argument to ftp_cwd, which is
initialized by the expression 'op_arg != NULL' in ftp_request().
Since I am requesting /All/emacs* that value seems correct to indicate
that the path requested for cwd (i.e., /All) is a subdirectory not a
file. However, here is the code that is executed within ftp_cwd
immediately after the printf() above:
/* Simple case: still in the home directory and no directory change. */
if (conn->ftp_home == NULL && strchr(path, '/') == NULL)
return 0;
Clearly, this condition holds and ftp_cwd immediately exits.
My analysis of this suggests, however, that only if subdir=0 should
this exit immediately. When there is not a '/' in the path, there are
two cases: (i) the path points to a file (i.e., subdir=0) or (ii) the
path points to the directory in which the file resides (i.e.,
subdir=1). In the example I have above, the second case holds and one
CWD command, not zero, should be emitted.
If all this logic is correct, I would expect something like the
following instead:
--- ftp.c
+++ ftp.c.orig
@@ -304,7 +304,7 @@
++path;
/* Simple case: still in the home directory and no directory change. */
- if (conn->ftp_home == NULL && strchr(path, '/') == NULL)
+ if (conn->ftp_home == NULL && strchr(path, '/') == NULL && !subdir)
return 0;
if ((e = ftp_cmd(conn, "PWD\r\n")) != FTP_WORKING_DIRECTORY ||
Indeed, that change makes this example complete correctly.
Since you are much more familiar with all of this code than I, please
verify that my logic is correct and that there are not other
implications of the change I suggest.
Thanks alot for taking a look at this.
Cheers,
Brook
From: Joerg Sonnenberger <joerg@britannica.bec.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/43013: Clarification of URL handling for pkg_add/pkgin
Date: Sat, 20 Mar 2010 13:40:54 +0100
On Sat, Mar 20, 2010 at 04:15:05AM +0000, Brook Milligan wrote:
> Clearly, this condition holds and ftp_cwd immediately exits.
Right, missed that edge case. Actually, it is should be
(!subdir || *path == '\0').
Joerg
From: Joerg Sonnenberger <joerg@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/43013 CVS commit: pkgsrc/net/libfetch
Date: Sun, 21 Mar 2010 16:48:43 +0000
Module Name: pkgsrc
Committed By: joerg
Date: Sun Mar 21 16:48:43 UTC 2010
Modified Files:
pkgsrc/net/libfetch: Makefile
pkgsrc/net/libfetch/files: common.c common.h ftp.c
Log Message:
libfetch-2.31:
PR 43013 by Brook Milligan: fetch(3) violates RFC 1738 for ftp:// URLs
if the home directory is not the root directory.
Remember the current directory the first time a CWD / CDUP has to be
issued. Use the document as full URL if the URL started with two /
(quoted or not), otherwise append it to the initial directory.
To generate a diff of this commit:
cvs rdiff -u -r1.37 -r1.38 pkgsrc/net/libfetch/Makefile
cvs rdiff -u -r1.25 -r1.26 pkgsrc/net/libfetch/files/common.c
cvs rdiff -u -r1.15 -r1.16 pkgsrc/net/libfetch/files/common.h
cvs rdiff -u -r1.34 -r1.35 pkgsrc/net/libfetch/files/ftp.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: Joerg Sonnenberger <joerg@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/43013 CVS commit: src/external/bsd/fetch/dist/libfetch
Date: Wed, 24 Mar 2010 20:51:46 +0000
Module Name: src
Committed By: joerg
Date: Wed Mar 24 20:51:46 UTC 2010
Update of /cvsroot/src/external/bsd/fetch/dist/libfetch
In directory ivanova.netbsd.org:/tmp/cvs-serv26424
Log Message:
libfetch-2.31:
PR 43013 by Brook Milligan: fetch(3) violates RFC 1738 for ftp:// URLs
if the home directory is not the root directory.
Remember the current directory the first time a CWD / CDUP has to be
issued. Use the document as full URL if the URL started with two /
(quoted or not), otherwise append it to the initial directory.
Status:
Vendor Tag: PKGSRC
Release Tags: libfetch-2-31
U src/external/bsd/fetch/dist/libfetch/common.c
U src/external/bsd/fetch/dist/libfetch/common.h
U src/external/bsd/fetch/dist/libfetch/errlist.sh
U src/external/bsd/fetch/dist/libfetch/fetch.3
U src/external/bsd/fetch/dist/libfetch/fetch.c
U src/external/bsd/fetch/dist/libfetch/fetch.h
U src/external/bsd/fetch/dist/libfetch/file.c
U src/external/bsd/fetch/dist/libfetch/ftp.c
U src/external/bsd/fetch/dist/libfetch/ftp.errors
U src/external/bsd/fetch/dist/libfetch/http.c
U src/external/bsd/fetch/dist/libfetch/http.errors
No conflicts created by this import
State-Changed-From-To: open->pending-pullups
State-Changed-By: joerg@NetBSD.org
State-Changed-When: Tue, 20 Apr 2010 00:59:34 +0000
State-Changed-Why:
Waiting on releng (#1351 for netbsd-5, #1390 for netbsd-4)
From: Jeff Rizzo <riz@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/43013 CVS commit: [netbsd-4] src/external/bsd/fetch/dist/libfetch
Date: Sun, 13 Jun 2010 05:59:11 +0000
Module Name: src
Committed By: riz
Date: Sun Jun 13 05:59:11 UTC 2010
Modified Files:
src/external/bsd/fetch/dist/libfetch [netbsd-4]: common.c common.h
ftp.c
Removed Files:
src/external/bsd/fetch/dist/libfetch [netbsd-4]: fetch.cat3
Log Message:
Pull up following revision(s) (requested by joerg in ticket #1390):
external/bsd/fetch/dist/libfetch/common.c: patch
external/bsd/fetch/dist/libfetch/common.h: patch
external/bsd/fetch/dist/libfetch/fetch.cat3: removed
external/bsd/fetch/dist/libfetch/ftp.c: patch
libfetch-2.31:
PR 43013 by Brook Milligan: fetch(3) violates RFC 1738 for ftp:// URLs
if the home directory is not the root directory.
Remember the current directory the first time a CWD / CDUP has to be
issued. Use the document as full URL if the URL started with two /
(quoted or not), otherwise append it to the initial directory.
To generate a diff of this commit:
cvs rdiff -u -r1.1.1.5.4.3 -r1.1.1.5.4.4 \
src/external/bsd/fetch/dist/libfetch/common.c
cvs rdiff -u -r1.1.1.4.4.3 -r1.1.1.4.4.4 \
src/external/bsd/fetch/dist/libfetch/common.h
cvs rdiff -u -r1.1.1.3.4.3 -r0 \
src/external/bsd/fetch/dist/libfetch/fetch.cat3
cvs rdiff -u -r1.1.1.6.4.3 -r1.1.1.6.4.4 \
src/external/bsd/fetch/dist/libfetch/ftp.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Jeff Rizzo" <riz@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/43013 CVS commit: [netbsd-5] src/external/bsd/fetch/dist/libfetch
Date: Sat, 20 Nov 2010 20:37:41 +0000
Module Name: src
Committed By: riz
Date: Sat Nov 20 20:37:41 UTC 2010
Modified Files:
src/external/bsd/fetch/dist/libfetch [netbsd-5]: common.c common.h
ftp.c
Log Message:
Pull up following revision(s) (requested by joerg in ticket #1351):
external/bsd/fetch/dist/libfetch/common.c: patch
external/bsd/fetch/dist/libfetch/common.h: patch
external/bsd/fetch/dist/libfetch/ftp.c: patch
update to libfetch-2.31:
PR 43013 by Brook Milligan: fetch(3) violates RFC 1738 for ftp:// URLs
if the home directory is not the root directory.
Remember the current directory the first time a CWD / CDUP has to be
issued. Use the document as full URL if the URL started with two /
(quoted or not), otherwise append it to the initial directory.
To generate a diff of this commit:
cvs rdiff -u -r1.1.1.2.4.2 -r1.1.1.2.4.3 \
src/external/bsd/fetch/dist/libfetch/common.c \
src/external/bsd/fetch/dist/libfetch/common.h
cvs rdiff -u -r1.1.1.3.2.2 -r1.1.1.3.2.3 \
src/external/bsd/fetch/dist/libfetch/ftp.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: pending-pullups->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sat, 20 Nov 2010 23:38:35 +0000
State-Changed-Why:
pullups complete.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home