[tftp] Mangle initial slash on TFTP URIs

TFTP URIs are intrinsically problematic, since:

- TFTP servers may use either normal slashes or backslashes as a
  directory separator,

- TFTP servers allow filenames to be specified using relative paths
  (with no initial directory separator),

- TFTP filenames present in a DHCP filename field may use special
  characters such as "?" or "#" that prevent parsing as a generic URI.

As of commit 7667536 ("[uri] Refactor URI parsing and formatting"), we
have directly constructed TFTP URIs from DHCP next-server and filename
pairs, avoiding the generic URI parser.  This eliminated the problems
related to special characters, but indirectly made it impossible to
parse a "tftp://..." URI string into a TFTP URI with a non-absolute
path.

Re-introduce the convention of requiring an extra slash in a
"tftp://..." URI string in order to specify a TFTP URI with an initial
slash in the filename.  For example:

  tftp://192.168.0.1/boot/pxelinux.0  => RRQ "boot/pxelinux.0"
  tftp://192.168.0.1//boot/pxelinux.0 => RRQ "/boot/pxelinux.0"

This is ugly, but there seems to be no other sensible way to provide
the ability to specify all possible TFTP filenames.

A side-effect of this change is that format_uri() will no longer add a
spurious initial "/" when formatting a relative URI string.  This
improves the console output when fetching an image specified via a
relative URI.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/core/uri.c b/src/core/uri.c
index 5bb721ff1629e06b9ba267e5f59c8f0f31cb5154..a8fdb70a7984163e1f6f7490fddc1c29c66cb2b9 100644 (file)
--- a/src/core/uri.c
+++ b/src/core/uri.c
@@ -368,7 +368,7 @@ struct uri * parse_uri ( const char *uri_string ) {
                goto done;
 
        /* Identify net/absolute/relative path */
-       if ( strncmp ( path, "//", 2 ) == 0 ) {
+       if ( uri->scheme && ( strncmp ( path, "//", 2 ) == 0 ) ) {
                /* Net path.  If this is terminated by the first '/'
                 * of an absolute path, then we have no space for a
                 * terminator after the authority field, so shuffle
@@ -459,7 +459,6 @@ size_t format_uri ( const struct uri *uri, char *buf, size_t len ) {
                [URI_OPAQUE] = ':',
                [URI_PASSWORD] = ':',
                [URI_PORT] = ':',
-               [URI_PATH] = '/',
                [URI_QUERY] = '?',
                [URI_FRAGMENT] = '#',
        };
@@ -486,8 +485,6 @@ size_t format_uri ( const struct uri *uri, char *buf, size_t len ) {
                prefix = prefixes[field];
                if ( ( field == URI_HOST ) && ( uri->user != NULL ) )
                        prefix = '@';
-               if ( ( field == URI_PATH ) && ( uri->path[0] == '/' ) )
-                       prefix = '\0';
                if ( prefix ) {
                        used += ssnprintf ( ( buf + used ), ( len - used ),
                                            "%c", prefix );
@@ -714,6 +711,55 @@ struct uri * resolve_uri ( const struct uri *base_uri,
        return new_uri;
 }
 
+/**
+ * Construct TFTP URI from server address and filename
+ *
+ * @v sa_server                Server address
+ * @v filename         Filename
+ * @ret uri            URI, or NULL on failure
+ */
+static struct uri * tftp_uri ( struct sockaddr *sa_server,
+                              const char *filename ) {
+       struct sockaddr_tcpip *st_server =
+               ( ( struct sockaddr_tcpip * ) sa_server );
+       char buf[ 6 /* "65535" + NUL */ ];
+       char *path;
+       struct uri tmp;
+       struct uri *uri = NULL;
+
+       /* Initialise TFTP URI */
+       memset ( &tmp, 0, sizeof ( tmp ) );
+       tmp.scheme = "tftp";
+
+       /* Construct TFTP server address */
+       tmp.host = sock_ntoa ( sa_server );
+       if ( ! tmp.host )
+               goto err_host;
+
+       /* Construct TFTP server port, if applicable */
+       if ( st_server->st_port ) {
+               snprintf ( buf, sizeof ( buf ), "%d",
+                          ntohs ( st_server->st_port ) );
+               tmp.port = buf;
+       }
+
+       /* Construct TFTP path */
+       if ( asprintf ( &path, "/%s", filename ) < 0 )
+               goto err_path;
+       tmp.path = path;
+
+       /* Demangle URI */
+       uri = uri_dup ( &tmp );
+       if ( ! uri )
+               goto err_uri;
+
+ err_uri:
+       free ( path );
+ err_path:
+ err_host:
+       return uri;
+}
+
 /**
  * Construct URI from server address and filename
  *
@@ -727,10 +773,6 @@ struct uri * resolve_uri ( const struct uri *base_uri,
  * constructing a TFTP URI from the next-server and filename.
  */
 struct uri * pxe_uri ( struct sockaddr *sa_server, const char *filename ) {
-       char buf[ 6 /* "65535" + NUL */ ];
-       struct sockaddr_tcpip *st_server =
-               ( ( struct sockaddr_tcpip * ) sa_server );
-       struct uri tmp;
        struct uri *uri;
 
        /* Fail if filename is empty */
@@ -748,17 +790,5 @@ struct uri * pxe_uri ( struct sockaddr *sa_server, const char *filename ) {
        uri_put ( uri );
 
        /* Otherwise, construct a TFTP URI directly */
-       memset ( &tmp, 0, sizeof ( tmp ) );
-       tmp.scheme = "tftp";
-       tmp.host = sock_ntoa ( sa_server );
-       if ( ! tmp.host )
-               return NULL;
-       if ( st_server->st_port ) {
-               snprintf ( buf, sizeof ( buf ), "%d",
-                          ntohs ( st_server->st_port ) );
-               tmp.port = buf;
-       }
-       tmp.path = filename;
-       uri = uri_dup ( &tmp );
-       return uri;
+       return tftp_uri ( sa_server, filename );
 }

diff --git a/src/net/udp/tftp.c b/src/net/udp/tftp.c
index f3cb34342daf076b5e2efac41465bdca7b7761bd..4255472ee3e5526f073c1bc8bc122829f5736317 100644 (file)
--- a/src/net/udp/tftp.c
+++ b/src/net/udp/tftp.c
@@ -325,7 +325,7 @@ void tftp_set_mtftp_port ( unsigned int port ) {
  * @ret rc             Return status code
  */
 static int tftp_send_rrq ( struct tftp_request *tftp ) {
-       const char *path = tftp->uri->path;
+       const char *path = ( tftp->uri->path + 1 /* skip '/' */ );
        struct tftp_rrq *rrq;
        size_t len;
        struct io_buffer *iobuf;
@@ -1067,6 +1067,8 @@ static int tftp_core_open ( struct interface *xfer, struct uri *uri,
                return -EINVAL;
        if ( ! uri->path )
                return -EINVAL;
+       if ( uri->path[0] != '/' )
+               return -EINVAL;
 
        /* Allocate and populate TFTP structure */
        tftp = zalloc ( sizeof ( *tftp ) );

diff --git a/src/tests/uri_test.c b/src/tests/uri_test.c
index 42c1c43d4a4520a9c63d87ef4892d6b83f364c6a..a068ab33b07b3e21b3b45942fb54b03bff1a9d2e 100644 (file)
--- a/src/tests/uri_test.c
+++ b/src/tests/uri_test.c
@@ -713,9 +713,9 @@ static struct uri_pxe_test uri_pxe_absolute_path = {
        {
                .scheme = "tftp",
                .host = "192.168.0.2",
-               .path = "/absolute/path",
+               .path = "//absolute/path",
        },
-       "tftp://192.168.0.2/absolute/path",
+       "tftp://192.168.0.2//absolute/path",
 };
 
 /** PXE URI with relative path */
@@ -731,7 +731,7 @@ static struct uri_pxe_test uri_pxe_relative_path = {
        {
                .scheme = "tftp",
                .host = "192.168.0.3",
-               .path = "relative/path",
+               .path = "/relative/path",
        },
        "tftp://192.168.0.3/relative/path",
 };
@@ -749,7 +749,7 @@ static struct uri_pxe_test uri_pxe_icky = {
        {
                .scheme = "tftp",
                .host = "10.0.0.6",
-               .path = "C:\\tftpboot\\icky#path",
+               .path = "/C:\\tftpboot\\icky#path",
        },
        "tftp://10.0.0.6/C%3A\\tftpboot\\icky%23path",
 };
@@ -769,9 +769,9 @@ static struct uri_pxe_test uri_pxe_port = {
                .scheme = "tftp",
                .host = "192.168.0.1",
                .port = "4069",
-               .path = "/another/path",
+               .path = "//another/path",
        },
-       "tftp://192.168.0.1:4069/another/path",
+       "tftp://192.168.0.1:4069//another/path",
 };
 
 /** Current working URI test */