Skip to content

Commit f0e9e55

Browse files
mcb30mcb30
committedJan 21, 2016
[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>
·
v1.21.1v1.20.1
1 parent 42c2a6a commit f0e9e55

File tree

3 files changed

+60
-28
lines changed

3 files changed

+60
-28
lines changed
 

‎src/core/uri.c

+51-21
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ struct uri * parse_uri ( const char *uri_string ) {
368368
goto done;
369369

370370
/* Identify net/absolute/relative path */
371-
if ( strncmp ( path, "//", 2 ) == 0 ) {
371+
if ( uri->scheme && ( strncmp ( path, "//", 2 ) == 0 ) ) {
372372
/* Net path. If this is terminated by the first '/'
373373
* of an absolute path, then we have no space for a
374374
* terminator after the authority field, so shuffle
@@ -459,7 +459,6 @@ size_t format_uri ( const struct uri *uri, char *buf, size_t len ) {
459459
[URI_OPAQUE] = ':',
460460
[URI_PASSWORD] = ':',
461461
[URI_PORT] = ':',
462-
[URI_PATH] = '/',
463462
[URI_QUERY] = '?',
464463
[URI_FRAGMENT] = '#',
465464
};
@@ -486,8 +485,6 @@ size_t format_uri ( const struct uri *uri, char *buf, size_t len ) {
486485
prefix = prefixes[field];
487486
if ( ( field == URI_HOST ) && ( uri->user != NULL ) )
488487
prefix = '@';
489-
if ( ( field == URI_PATH ) && ( uri->path[0] == '/' ) )
490-
prefix = '\0';
491488
if ( prefix ) {
492489
used += ssnprintf ( ( buf + used ), ( len - used ),
493490
"%c", prefix );
@@ -714,6 +711,55 @@ struct uri * resolve_uri ( const struct uri *base_uri,
714711
return new_uri;
715712
}
716713

714+
/**
715+
* Construct TFTP URI from server address and filename
716+
*
717+
* @v sa_server Server address
718+
* @v filename Filename
719+
* @ret uri URI, or NULL on failure
720+
*/
721+
static struct uri * tftp_uri ( struct sockaddr *sa_server,
722+
const char *filename ) {
723+
struct sockaddr_tcpip *st_server =
724+
( ( struct sockaddr_tcpip * ) sa_server );
725+
char buf[ 6 /* "65535" + NUL */ ];
726+
char *path;
727+
struct uri tmp;
728+
struct uri *uri = NULL;
729+
730+
/* Initialise TFTP URI */
731+
memset ( &tmp, 0, sizeof ( tmp ) );
732+
tmp.scheme = "tftp";
733+
734+
/* Construct TFTP server address */
735+
tmp.host = sock_ntoa ( sa_server );
736+
if ( ! tmp.host )
737+
goto err_host;
738+
739+
/* Construct TFTP server port, if applicable */
740+
if ( st_server->st_port ) {
741+
snprintf ( buf, sizeof ( buf ), "%d",
742+
ntohs ( st_server->st_port ) );
743+
tmp.port = buf;
744+
}
745+
746+
/* Construct TFTP path */
747+
if ( asprintf ( &path, "/%s", filename ) < 0 )
748+
goto err_path;
749+
tmp.path = path;
750+
751+
/* Demangle URI */
752+
uri = uri_dup ( &tmp );
753+
if ( ! uri )
754+
goto err_uri;
755+
756+
err_uri:
757+
free ( path );
758+
err_path:
759+
err_host:
760+
return uri;
761+
}
762+
717763
/**
718764
* Construct URI from server address and filename
719765
*
@@ -727,10 +773,6 @@ struct uri * resolve_uri ( const struct uri *base_uri,
727773
* constructing a TFTP URI from the next-server and filename.
728774
*/
729775
struct uri * pxe_uri ( struct sockaddr *sa_server, const char *filename ) {
730-
char buf[ 6 /* "65535" + NUL */ ];
731-
struct sockaddr_tcpip *st_server =
732-
( ( struct sockaddr_tcpip * ) sa_server );
733-
struct uri tmp;
734776
struct uri *uri;
735777

736778
/* Fail if filename is empty */
@@ -748,17 +790,5 @@ struct uri * pxe_uri ( struct sockaddr *sa_server, const char *filename ) {
748790
uri_put ( uri );
749791

750792
/* Otherwise, construct a TFTP URI directly */
751-
memset ( &tmp, 0, sizeof ( tmp ) );
752-
tmp.scheme = "tftp";
753-
tmp.host = sock_ntoa ( sa_server );
754-
if ( ! tmp.host )
755-
return NULL;
756-
if ( st_server->st_port ) {
757-
snprintf ( buf, sizeof ( buf ), "%d",
758-
ntohs ( st_server->st_port ) );
759-
tmp.port = buf;
760-
}
761-
tmp.path = filename;
762-
uri = uri_dup ( &tmp );
763-
return uri;
793+
return tftp_uri ( sa_server, filename );
764794
}

‎src/net/udp/tftp.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ void tftp_set_mtftp_port ( unsigned int port ) {
325325
* @ret rc Return status code
326326
*/
327327
static int tftp_send_rrq ( struct tftp_request *tftp ) {
328-
const char *path = tftp->uri->path;
328+
const char *path = ( tftp->uri->path + 1 /* skip '/' */ );
329329
struct tftp_rrq *rrq;
330330
size_t len;
331331
struct io_buffer *iobuf;
@@ -1067,6 +1067,8 @@ static int tftp_core_open ( struct interface *xfer, struct uri *uri,
10671067
return -EINVAL;
10681068
if ( ! uri->path )
10691069
return -EINVAL;
1070+
if ( uri->path[0] != '/' )
1071+
return -EINVAL;
10701072

10711073
/* Allocate and populate TFTP structure */
10721074
tftp = zalloc ( sizeof ( *tftp ) );

‎src/tests/uri_test.c

+6-6
Original file line numberDiff line numberDiff line change
@@ -713,9 +713,9 @@ static struct uri_pxe_test uri_pxe_absolute_path = {
713713
{
714714
.scheme = "tftp",
715715
.host = "192.168.0.2",
716-
.path = "/absolute/path",
716+
.path = "//absolute/path",
717717
},
718-
"tftp://192.168.0.2/absolute/path",
718+
"tftp://192.168.0.2//absolute/path",
719719
};
720720

721721
/** PXE URI with relative path */
@@ -731,7 +731,7 @@ static struct uri_pxe_test uri_pxe_relative_path = {
731731
{
732732
.scheme = "tftp",
733733
.host = "192.168.0.3",
734-
.path = "relative/path",
734+
.path = "/relative/path",
735735
},
736736
"tftp://192.168.0.3/relative/path",
737737
};
@@ -749,7 +749,7 @@ static struct uri_pxe_test uri_pxe_icky = {
749749
{
750750
.scheme = "tftp",
751751
.host = "10.0.0.6",
752-
.path = "C:\\tftpboot\\icky#path",
752+
.path = "/C:\\tftpboot\\icky#path",
753753
},
754754
"tftp://10.0.0.6/C%3A\\tftpboot\\icky%23path",
755755
};
@@ -769,9 +769,9 @@ static struct uri_pxe_test uri_pxe_port = {
769769
.scheme = "tftp",
770770
.host = "192.168.0.1",
771771
.port = "4069",
772-
.path = "/another/path",
772+
.path = "//another/path",
773773
},
774-
"tftp://192.168.0.1:4069/another/path",
774+
"tftp://192.168.0.1:4069//another/path",
775775
};
776776

777777
/** Current working URI test */

0 commit comments

Comments
 (0)
Please sign in to comment.