Skip to content

Commit

Permalink
[tftp] Mangle initial slash on TFTP URIs
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
mcb30 committed Jan 21, 2016
1 parent 42c2a6a commit f0e9e55
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 28 deletions.
72 changes: 51 additions & 21 deletions src/core/uri.c
Expand Up @@ -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
Expand Down Expand Up @@ -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] = '#',
};
Expand All @@ -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 );
Expand Down Expand Up @@ -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
*
Expand All @@ -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 */
Expand All @@ -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 );
}
4 changes: 3 additions & 1 deletion src/net/udp/tftp.c
Expand Up @@ -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;
Expand Down Expand Up @@ -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 ) );
Expand Down
12 changes: 6 additions & 6 deletions src/tests/uri_test.c
Expand Up @@ -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 */
Expand All @@ -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",
};
Expand All @@ -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",
};
Expand All @@ -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 */
Expand Down

0 comments on commit f0e9e55

Please sign in to comment.