From 3f4de99f16ac819f292ca568095d7032d88e5a8b Mon Sep 17 00:00:00 2001 From: Witold Filipczyk Date: Sat, 27 Jan 2007 10:05:40 +0100 Subject: [PATCH 01/11] ftp: ftp didn't handle filenames with spaces. --- src/protocol/common.c | 8 ++------ src/protocol/ftp/ftp.c | 26 +++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/protocol/common.c b/src/protocol/common.c index c21dc42ca..0c985ba17 100644 --- a/src/protocol/common.c +++ b/src/protocol/common.c @@ -63,12 +63,8 @@ init_directory_listing(struct string *page, struct uri *uri) && !add_char_to_string(&dirpath, local ? CHAR_DIR_SEP : '/')) goto out_of_memory; - if (local || uri->protocol == PROTOCOL_FSP || uri->protocol == PROTOCOL_GOPHER - || uri->protocol == PROTOCOL_SMB) { - /* A little hack to get readable Gopher names. We should find a - * way to do it more general. */ - decode_uri_string(&dirpath); - } + /* Decode uri for displaying. */ + decode_uri_string(&dirpath); if (!local && !add_char_to_string(&location, '/')) goto out_of_memory; diff --git a/src/protocol/ftp/ftp.c b/src/protocol/ftp/ftp.c index 01e301bc0..ac2917a07 100644 --- a/src/protocol/ftp/ftp.c +++ b/src/protocol/ftp/ftp.c @@ -683,8 +683,15 @@ add_file_cmd_to_str(struct connection *conn) if (!conn->uri->datalen || conn->uri->data[conn->uri->datalen - 1] == '/') { + struct string uri_string; /* Commands to get directory listing. */ + if (!init_string(&uri_string)) { + done_string(&command); + done_string(&ftp_data_command); + abort_connection(conn, S_OUT_OF_MEM); + return NULL; + } ftp->dir = 1; ftp->pending_commands = 4; @@ -695,17 +702,27 @@ add_file_cmd_to_str(struct connection *conn) add_string_to_string(&command, &ftp_data_command); add_to_string(&command, "CWD "); - add_uri_to_string(&command, conn->uri, URI_PATH); + add_uri_to_string(&uri_string, conn->uri, URI_PATH); + decode_uri_string(&uri_string); + add_string_to_string(&command, &uri_string); add_crlf_to_string(&command); add_to_string(&command, "LIST"); add_crlf_to_string(&command); + done_string(&uri_string); conn->from = 0; } else { + struct string uri_string; /* Commands to get a file. */ + if (!init_string(&uri_string)) { + done_string(&command); + done_string(&ftp_data_command); + abort_connection(conn, S_OUT_OF_MEM); + return NULL; + } ftp->dir = 0; ftp->pending_commands = 3; @@ -727,8 +744,11 @@ add_file_cmd_to_str(struct connection *conn) } add_to_string(&command, "RETR "); - add_uri_to_string(&command, conn->uri, URI_PATH); + add_uri_to_string(&uri_string, conn->uri, URI_PATH); + decode_uri_string(&uri_string); + add_string_to_string(&command, &uri_string); add_crlf_to_string(&command); + done_string(&uri_string); } done_string(&ftp_data_command); @@ -1134,7 +1154,7 @@ display_dir_entry(struct cache_entry *cached, off_t *pos, int *tries, } add_to_string(&string, "name.source, ftp_info->name.length); + encode_uri_string(&string, ftp_info->name.source, ftp_info->name.length, 0); if (ftp_info->type == FTP_FILE_DIRECTORY) add_char_to_string(&string, '/'); add_to_string(&string, "\">"); From adfde652b99c297bc05fd4876249669cc34a01ad Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sun, 18 Mar 2007 08:53:56 +0200 Subject: [PATCH 02/11] Reject CR and LF characters in FTP pathnames. --- src/protocol/ftp/ftp.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/protocol/ftp/ftp.c b/src/protocol/ftp/ftp.c index ac2917a07..284579541 100644 --- a/src/protocol/ftp/ftp.c +++ b/src/protocol/ftp/ftp.c @@ -638,6 +638,26 @@ get_ftp_data_socket(struct connection *conn, struct string *command) return 1; } +/* Check if the file or directory name @s can be safely sent to the + * FTP server. To prevent command injection attacks, this function + * must reject CR LF sequences. */ +static int +is_ftp_pathname_safe(const struct string *s) +{ + int i; + + /* RFC 959 says the argument of CWD and RETR is a , + * which consists of s, "any of the 128 ASCII characters + * except and ". So other control characters, such + * as 0x00 and 0x7F, are allowed here. Bytes 0x80...0xFF + * should not be allowed, but if we reject them, users will + * probably complain. */ + for (i = 0; i < s->length; i++) { + if (s->source[i] == 0x0A || s->source[i] == 0x0D) + return 0; + } + return 1; +} /* Create passive socket and add appropriate announcing commands to str. Then * go and retrieve appropriate object from server. @@ -704,6 +724,13 @@ add_file_cmd_to_str(struct connection *conn) add_to_string(&command, "CWD "); add_uri_to_string(&uri_string, conn->uri, URI_PATH); decode_uri_string(&uri_string); + if (!is_ftp_pathname_safe(&uri_string)) { + done_string(&uri_string); + done_string(&command); + done_string(&ftp_data_command); + abort_connection(conn, S_BAD_URL); + return NULL; + } add_string_to_string(&command, &uri_string); add_crlf_to_string(&command); @@ -746,6 +773,13 @@ add_file_cmd_to_str(struct connection *conn) add_to_string(&command, "RETR "); add_uri_to_string(&uri_string, conn->uri, URI_PATH); decode_uri_string(&uri_string); + if (!is_ftp_pathname_safe(&uri_string)) { + done_string(&uri_string); + done_string(&command); + done_string(&ftp_data_command); + abort_connection(conn, S_BAD_URL); + return NULL; + } add_string_to_string(&command, &uri_string); add_crlf_to_string(&command); done_string(&uri_string); From c6b8fa71511fa84a01ca8fa73e494427c7732742 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sun, 18 Mar 2007 09:28:08 +0200 Subject: [PATCH 03/11] ftp, add_file_cmd_to_str: Check errors from string functions. Use goto for error handling. Free all strings in the same place. --- src/protocol/ftp/ftp.c | 146 ++++++++++++++++++++--------------------- 1 file changed, 70 insertions(+), 76 deletions(-) diff --git a/src/protocol/ftp/ftp.c b/src/protocol/ftp/ftp.c index 284579541..044af97ce 100644 --- a/src/protocol/ftp/ftp.c +++ b/src/protocol/ftp/ftp.c @@ -665,143 +665,137 @@ is_ftp_pathname_safe(const struct string *s) static struct ftp_connection_info * add_file_cmd_to_str(struct connection *conn) { - struct ftp_connection_info *ftp; - struct string command, ftp_data_command; + int ok = 0; + struct ftp_connection_info *ftp = NULL; + struct string command = NULL_STRING; + struct string ftp_data_command = NULL_STRING; + struct string pathname = NULL_STRING; if (!conn->uri->data) { INTERNAL("conn->uri->data empty"); abort_connection(conn, S_INTERNAL); - return NULL; + goto ret; } + /* This will be reallocated below when we know how long the + * command string should be. Error handling could be + * simplified a little by allocating this initial structure on + * the stack, but it's several kilobytes long so that might be + * risky. */ ftp = mem_calloc(1, sizeof(*ftp)); if (!ftp) { abort_connection(conn, S_OUT_OF_MEM); - return NULL; + goto ret; } conn->info = ftp; /* Freed when connection is destroyed. */ - if (!init_string(&command)) { + if (!init_string(&command) + || !init_string(&ftp_data_command) + || !init_string(&pathname)) { abort_connection(conn, S_OUT_OF_MEM); - return NULL; - } - - if (!init_string(&ftp_data_command)) { - done_string(&command); - abort_connection(conn, S_OUT_OF_MEM); - return NULL; + goto ret; } if (!get_ftp_data_socket(conn, &ftp_data_command)) { - done_string(&command); - done_string(&ftp_data_command); INTERNAL("Ftp data socket failure"); abort_connection(conn, S_INTERNAL); - return NULL; + goto ret; + } + + if (!add_uri_to_string(&pathname, conn->uri, URI_PATH)) { + abort_connection(conn, S_OUT_OF_MEM); + goto ret; + } + + decode_uri_string(&pathname); + if (!is_ftp_pathname_safe(&pathname)) { + abort_connection(conn, S_BAD_URL); + goto ret; } if (!conn->uri->datalen || conn->uri->data[conn->uri->datalen - 1] == '/') { - struct string uri_string; /* Commands to get directory listing. */ - if (!init_string(&uri_string)) { - done_string(&command); - done_string(&ftp_data_command); - abort_connection(conn, S_OUT_OF_MEM); - return NULL; - } ftp->dir = 1; ftp->pending_commands = 4; - /* ASCII */ - add_to_string(&command, "TYPE A"); - add_crlf_to_string(&command); + if (!add_to_string(&command, "TYPE A") /* ASCII */ + || !add_crlf_to_string(&command) - add_string_to_string(&command, &ftp_data_command); + || !add_string_to_string(&command, &ftp_data_command) + + || !add_to_string(&command, "CWD ") + || !add_string_to_string(&command, &pathname) + || !add_crlf_to_string(&command) - add_to_string(&command, "CWD "); - add_uri_to_string(&uri_string, conn->uri, URI_PATH); - decode_uri_string(&uri_string); - if (!is_ftp_pathname_safe(&uri_string)) { - done_string(&uri_string); - done_string(&command); - done_string(&ftp_data_command); - abort_connection(conn, S_BAD_URL); - return NULL; + || !add_to_string(&command, "LIST") + || !add_crlf_to_string(&command)) { + abort_connection(conn, S_OUT_OF_MEM); + goto ret; } - add_string_to_string(&command, &uri_string); - add_crlf_to_string(&command); - - add_to_string(&command, "LIST"); - add_crlf_to_string(&command); - done_string(&uri_string); conn->from = 0; } else { - struct string uri_string; /* Commands to get a file. */ - if (!init_string(&uri_string)) { - done_string(&command); - done_string(&ftp_data_command); - abort_connection(conn, S_OUT_OF_MEM); - return NULL; - } ftp->dir = 0; ftp->pending_commands = 3; - /* BINARY */ - add_to_string(&command, "TYPE I"); - add_crlf_to_string(&command); + if (!add_to_string(&command, "TYPE I") /* BINARY */ + || !add_crlf_to_string(&command) - add_string_to_string(&command, &ftp_data_command); + || !add_string_to_string(&command, &ftp_data_command)) { + abort_connection(conn, S_OUT_OF_MEM); + goto ret; + } if (conn->from || conn->progress->start > 0) { - add_to_string(&command, "REST "); - add_long_to_string(&command, conn->from - ? conn->from - : conn->progress->start); - add_crlf_to_string(&command); + const off_t offset = conn->from + ? conn->from + : conn->progress->start; + + if (!add_to_string(&command, "REST ") + || !add_long_to_string(&command, offset) + || !add_crlf_to_string(&command)) { + abort_connection(conn, S_OUT_OF_MEM); + goto ret; + } ftp->rest_sent = 1; ftp->pending_commands++; } - add_to_string(&command, "RETR "); - add_uri_to_string(&uri_string, conn->uri, URI_PATH); - decode_uri_string(&uri_string); - if (!is_ftp_pathname_safe(&uri_string)) { - done_string(&uri_string); - done_string(&command); - done_string(&ftp_data_command); - abort_connection(conn, S_BAD_URL); - return NULL; + if (!add_to_string(&command, "RETR ") + || !add_string_to_string(&command, &pathname) + || !add_crlf_to_string(&command)) { + abort_connection(conn, S_OUT_OF_MEM); + goto ret; } - add_string_to_string(&command, &uri_string); - add_crlf_to_string(&command); - done_string(&uri_string); } - done_string(&ftp_data_command); - ftp->opc = ftp->pending_commands; /* 1 byte is already reserved for cmd_buffer in struct ftp_connection_info. */ ftp = mem_realloc(ftp, sizeof(*ftp) + command.length); if (!ftp) { - done_string(&command); abort_connection(conn, S_OUT_OF_MEM); - return NULL; + goto ret; } + conn->info = ftp; /* in case mem_realloc moved the buffer */ memcpy(ftp->cmd_buffer, command.source, command.length + 1); - done_string(&command); - conn->info = ftp; + ok = 1; - return ftp; +ret: + /* If @ok is false here, then abort_connection has already + * freed @ftp, which now is a dangling pointer. */ + done_string(&pathname); + done_string(&ftp_data_command); + done_string(&command); + return ok ? ftp : NULL; } static void From b48a2d660dc88258f59bba4134e0e46512e51737 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sun, 18 Mar 2007 09:56:43 +0200 Subject: [PATCH 04/11] More const in URI functions. Not yet all of them, though. --- src/protocol/uri.c | 30 ++++++++++++++++-------------- src/protocol/uri.h | 15 +++++++++------ 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/protocol/uri.c b/src/protocol/uri.c index 634688b5e..df226464c 100644 --- a/src/protocol/uri.c +++ b/src/protocol/uri.c @@ -47,14 +47,14 @@ end_of_dir(unsigned char c) } static inline int -is_uri_dir_sep(struct uri *uri, unsigned char pos) +is_uri_dir_sep(const struct uri *uri, unsigned char pos) { return (uri->protocol == PROTOCOL_FILE ? dir_sep(pos) : pos == '/'); } int -is_ip_address(unsigned char *address, int addresslen) +is_ip_address(const unsigned char *address, int addresslen) { /* The @address has well defined limits so it would be a shame to * allocate it. */ @@ -90,7 +90,7 @@ is_ip_address(unsigned char *address, int addresslen) int -end_with_known_tld(unsigned char *s, int slen) +end_with_known_tld(const unsigned char *s, int slen) { int i; static const unsigned char *const tld[] = @@ -146,7 +146,7 @@ check_whether_file_exists(unsigned char *name) } static int -check_uri_file(unsigned char *name) +check_uri_file(const unsigned char *name) { /* Check POST_CHAR etc ... */ static const unsigned char chars[] = POST_CHAR_S "#?"; @@ -409,10 +409,10 @@ parse_uri(struct uri *uri, unsigned char *uristring) } int -get_uri_port(struct uri *uri) +get_uri_port(const struct uri *uri) { if (uri->port && uri->portlen) { - unsigned char *end = uri->port; + const unsigned char *end = uri->port; int port = strtol(uri->port, (char **) &end, 10); if (end != uri->port) { @@ -427,7 +427,8 @@ get_uri_port(struct uri *uri) #define can_compare_uri_components(comp) !(((comp) & (URI_SPECIAL | URI_IDN))) static inline int -compare_component(unsigned char *a, int alen, unsigned char *b, int blen) +compare_component(const unsigned char *a, int alen, + const unsigned char *b, int blen) { /* Check that the length and the strings are both set or unset */ if (alen != blen || !!a != !!b) return 0; @@ -442,7 +443,8 @@ compare_component(unsigned char *a, int alen, unsigned char *b, int blen) #define wants(x) (components & (x)) int -compare_uri(struct uri *a, struct uri *b, enum uri_component components) +compare_uri(const struct uri *a, const struct uri *b, + enum uri_component components) { if (a == b) return 1; if (!components) return 0; @@ -471,7 +473,7 @@ compare_uri(struct uri *a, struct uri *b, enum uri_component components) /* We might need something more intelligent than this Swiss army knife. */ struct string * -add_uri_to_string(struct string *string, struct uri *uri, +add_uri_to_string(struct string *string, const struct uri *uri, enum uri_component components) { /* Custom or unknown keep the URI untouched. */ @@ -578,8 +580,8 @@ add_uri_to_string(struct string *string, struct uri *uri, /* We can not test uri->datalen here since we need to always * add '/'. */ if (wants(URI_PATH) || wants(URI_FILENAME)) { - unsigned char *filename = uri->data; - unsigned char *pos; + const unsigned char *filename = uri->data; + const unsigned char *pos; assertm(!wants(URI_FILENAME) || components == URI_FILENAME, "URI_FILENAME should be used alone %d", components); @@ -602,7 +604,7 @@ add_uri_to_string(struct string *string, struct uri *uri, } if (wants(URI_QUERY) && uri->datalen) { - unsigned char *query = memchr(uri->data, '?', uri->datalen); + const unsigned char *query = memchr(uri->data, '?', uri->datalen); assertm(URI_QUERY == components, "URI_QUERY should be used alone %d", components); @@ -642,7 +644,7 @@ add_uri_to_string(struct string *string, struct uri *uri, #undef wants unsigned char * -get_uri_string(struct uri *uri, enum uri_component components) +get_uri_string(const struct uri *uri, enum uri_component components) { struct string string; @@ -785,7 +787,7 @@ normalize_uri(struct uri *uri, unsigned char *uristring) * backend can understand. No host parts etc, that is what this function is * supposed to chew. */ static struct uri * -transform_file_url(struct uri *uri, unsigned char *cwd) +transform_file_url(struct uri *uri, const unsigned char *cwd) { unsigned char *path = uri->data; diff --git a/src/protocol/uri.h b/src/protocol/uri.h index b3eeecc4e..426bd11c8 100644 --- a/src/protocol/uri.h +++ b/src/protocol/uri.h @@ -247,25 +247,28 @@ unsigned char *normalize_uri(struct uri *uri, unsigned char *uristring); /* Check if two URIs are equal. If @components are 0 simply compare the whole * URI else only compare the specific parts. */ -int compare_uri(struct uri *uri1, struct uri *uri2, enum uri_component components); +int compare_uri(const struct uri *uri1, const struct uri *uri2, + enum uri_component components); /* These functions recreate the URI string part by part. */ /* The @components bitmask describes the set of URI components used for * construction of the URI string. */ /* Adds the components to an already initialized string. */ -struct string *add_uri_to_string(struct string *string, struct uri *uri, enum uri_component components); +struct string *add_uri_to_string(struct string *string, const struct uri *uri, + enum uri_component components); /* Takes an uri string, parses it and adds the desired components. Useful if * there is no struct uri around. */ struct string *add_string_uri_to_string(struct string *string, unsigned char *uristring, enum uri_component components); /* Returns the new URI string or NULL upon an error. */ -unsigned char *get_uri_string(struct uri *uri, enum uri_component components); +unsigned char *get_uri_string(const struct uri *uri, + enum uri_component components); /* Returns either the uri's port number if available or the protocol's * default port. It is zarro for user protocols. */ -int get_uri_port(struct uri *uri); +int get_uri_port(const struct uri *uri); /* Tcp port range */ #define LOWEST_PORT 0 @@ -304,7 +307,7 @@ unsigned char *join_urls(struct uri *base, unsigned char *relative); /* Return position if end of string @s matches a known tld or -1 if not. * If @slen < 0, then string length will be obtained by a strlen() call, * else @slen is used as @s length. */ -int end_with_known_tld(unsigned char *s, int slen); +int end_with_known_tld(const unsigned char *s, int slen); static inline int @@ -314,6 +317,6 @@ get_real_uri_length(struct uri *uri) } /* Checks if @address contains a valid IP address. */ -int is_ip_address(unsigned char *address, int addresslen); +int is_ip_address(const unsigned char *address, int addresslen); #endif From 3ee04479d44f5a693f672a72b0b65dcfc0d78128 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sun, 18 Mar 2007 11:04:02 +0200 Subject: [PATCH 05/11] init_directory_listing: Link to original URLs. Decode for display only. HTML-escape all strings that are not intended to contain markup. --- src/protocol/common.c | 51 +++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/src/protocol/common.c b/src/protocol/common.c index 0c985ba17..d22fd84b1 100644 --- a/src/protocol/common.c +++ b/src/protocol/common.c @@ -47,12 +47,14 @@ enum connection_state init_directory_listing(struct string *page, struct uri *uri) { struct string dirpath = NULL_STRING; + struct string decoded = NULL_STRING; struct string location = NULL_STRING; unsigned char *info; int local = (uri->protocol == PROTOCOL_FILE); if (!init_string(page) || !init_string(&dirpath) + || !init_string(&decoded) || !init_string(&location) || !add_uri_to_string(&dirpath, uri, URI_DATA) || !add_uri_to_string(&location, uri, URI_DIR_LOCATION)) @@ -63,8 +65,12 @@ init_directory_listing(struct string *page, struct uri *uri) && !add_char_to_string(&dirpath, local ? CHAR_DIR_SEP : '/')) goto out_of_memory; - /* Decode uri for displaying. */ - decode_uri_string(&dirpath); + /* Decode uri for displaying. Do not use + * add_string_to_string, because it for some reason returns + * NULL if the second string is empty. */ + if (!add_bytes_to_string(&decoded, dirpath.source, dirpath.length)) + goto out_of_memory; + decode_uri_string(&decoded); if (!local && !add_char_to_string(&location, '/')) goto out_of_memory; @@ -72,23 +78,15 @@ init_directory_listing(struct string *page, struct uri *uri) if (!add_to_string(page, "\n")) goto out_of_memory; - if (!local && !add_string_to_string(page, &location)) + if (!local && !add_html_to_string(page, location.source, location.length)) goto out_of_memory; - if (!add_html_to_string(page, dirpath.source, dirpath.length) + if (!add_html_to_string(page, decoded.source, decoded.length) || !add_to_string(page, "\n\n\n\n

")) goto out_of_memory; @@ -122,20 +120,24 @@ init_directory_listing(struct string *page, struct uri *uri) /* Make the directory path with links to each subdir. */ { - unsigned char *slash = dirpath.source; - unsigned char *pslash = slash; - unsigned char sep = local ? CHAR_DIR_SEP : '/'; + const unsigned char *slash = dirpath.source; + const unsigned char *pslash = slash; + const unsigned char sep = local ? CHAR_DIR_SEP : '/'; + + while ((slash = strchr(slash, sep)) != NULL) { + done_string(&decoded); + if (!init_string(&decoded) + || !add_bytes_to_string(&decoded, pslash, slash - pslash)) + goto out_of_memory; + decode_uri_string(&decoded); - while ((slash = strchr(slash, sep))) { - /* FIXME: htmlesc? At least we should escape quotes. --pasky */ if (!add_to_string(page, "") - || !add_html_to_string(page, pslash, slash - pslash) + || !add_html_to_string(page, decoded.source, decoded.length) || !add_to_string(page, "") - || !add_char_to_string(page, sep)) + || !add_html_to_string(page, &sep, 1)) goto out_of_memory; pslash = ++slash; @@ -148,6 +150,7 @@ out_of_memory: } done_string(&dirpath); + done_string(&decoded); done_string(&location); return page->length > 0 ? S_OK : S_OUT_OF_MEM; From 36dd1fa267575d717e7cf5c75af698c34f55b8b8 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sun, 18 Mar 2007 11:10:33 +0200 Subject: [PATCH 06/11] add_html_to_string: Encode only known unsafe or non-ASCII characters. In particular, do not encode '%' and '/', which are common in URIs. --- src/util/conv.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/util/conv.c b/src/util/conv.c index 5a43dbc65..c7ddd66c2 100644 --- a/src/util/conv.c +++ b/src/util/conv.c @@ -274,22 +274,18 @@ add_string_replace(struct string *string, unsigned char *src, int len, struct string * add_html_to_string(struct string *string, const unsigned char *src, int len) { - -#define isalphanum(q) (isalnum(q) || (q) == '-' || (q) == '_') - for (; len; len--, src++) { - if (isalphanum(*src) || *src == ' ' - || *src == '.' || *src == ':' || *src == ';') { - add_bytes_to_string(string, src, 1); - } else { + if (*src < 0x20 || *src >= 0x7F + || *src == '<' || *src == '>' || *src == '&' + || *src == '\"' || *src == '\'') { add_bytes_to_string(string, "&#", 2); add_long_to_string(string, (long) *src); add_char_to_string(string, ';'); + } else { + add_char_to_string(string, *src); } } -#undef isalphanum - return string; } From 28645973e564a3e3018c00dcd4023e9bfcec471b Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sun, 18 Mar 2007 11:13:38 +0200 Subject: [PATCH 07/11] add_html_to_string: If out of memory, roll back and return NULL. --- src/util/conv.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/util/conv.c b/src/util/conv.c index c7ddd66c2..6447ed93e 100644 --- a/src/util/conv.c +++ b/src/util/conv.c @@ -278,11 +278,18 @@ add_html_to_string(struct string *string, const unsigned char *src, int len) if (*src < 0x20 || *src >= 0x7F || *src == '<' || *src == '>' || *src == '&' || *src == '\"' || *src == '\'') { - add_bytes_to_string(string, "&#", 2); - add_long_to_string(string, (long) *src); - add_char_to_string(string, ';'); + int rollback_length = string->length; + + if (!add_bytes_to_string(string, "&#", 2) + || !add_long_to_string(string, (long) *src) + || !add_char_to_string(string, ';')) { + string->length = rollback_length; + return NULL; + } + } else { - add_char_to_string(string, *src); + if (!add_char_to_string(string, *src)) + return NULL; } } From b7dddaa6853a404070265a2cfb8535e90f265801 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sun, 18 Mar 2007 12:36:33 +0200 Subject: [PATCH 08/11] add_html_to_string: Also restore the '\0' terminator. --- src/util/conv.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/conv.c b/src/util/conv.c index 6447ed93e..5beb001f3 100644 --- a/src/util/conv.c +++ b/src/util/conv.c @@ -284,6 +284,7 @@ add_html_to_string(struct string *string, const unsigned char *src, int len) || !add_long_to_string(string, (long) *src) || !add_char_to_string(string, ';')) { string->length = rollback_length; + string->source[rollback_length] = '\0'; return NULL; } From 682ad62a974490246a7387a8bbdf197108c71137 Mon Sep 17 00:00:00 2001 From: Witold Filipczyk Date: Sat, 10 Feb 2007 22:12:27 +0100 Subject: [PATCH 09/11] Accesskey didn't work as it should. [ From commit 5008fb697d93569984e2163c528c6404d36bcfa9 on the witekfl branch. --KON ] --- src/viewer/text/link.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/viewer/text/link.c b/src/viewer/text/link.c index 7a69a9ce7..2e1ae52c6 100644 --- a/src/viewer/text/link.c +++ b/src/viewer/text/link.c @@ -1197,7 +1197,6 @@ try_document_key(struct session *ses, struct document_view *doc_view, struct term_event *ev) { unicode_val_T key; - int passed = -1; int i; /* GOD I HATE C! --FF */ /* YEAH, BRAINFUCK RULEZ! --pasky */ assert(ses && doc_view && doc_view->document && doc_view->vs && ev); @@ -1224,24 +1223,24 @@ try_document_key(struct session *ses, struct document_view *doc_view, /* Run through all the links and see if one of them is bound to the * key we test.. */ - for (i = 0; i < doc_view->document->nlinks; i++) { + + i = doc_view->vs->current_link >= 0 ? doc_view->vs->current_link : 0; + for (; i < doc_view->document->nlinks; i++) { struct link *link = &doc_view->document->links[i]; if (key == link->accesskey) { - if (passed != i && i <= doc_view->vs->current_link) { - /* This is here in order to rotate between - * links with same accesskey. */ - if (passed < 0) passed = i; - continue; - } ses->kbdprefix.repeat_count = 0; goto_link_number_do(ses, doc_view, i); return FRAME_EVENT_REFRESH; } + } + for (i = 0; i < doc_view->vs->current_link; i++) { + struct link *link = &doc_view->document->links[i]; - if (i == doc_view->document->nlinks - 1 && passed >= 0) { - /* Return to the start. */ - i = passed - 1; + if (key == link->accesskey) { + ses->kbdprefix.repeat_count = 0; + goto_link_number_do(ses, doc_view, i); + return FRAME_EVENT_REFRESH; } } From d2970e57af189164275f334b8d0d5d505294fcc7 Mon Sep 17 00:00:00 2001 From: Witold Filipczyk Date: Sun, 11 Feb 2007 09:21:35 +0100 Subject: [PATCH 10/11] accesskey: start iterating with next link. Rotating between links with the same accesskey works. [ From commit c2d1952a082e2ed51dbfb6895f29c0869a89a8a3 on the witekfl branch. --KON ] --- src/viewer/text/link.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/viewer/text/link.c b/src/viewer/text/link.c index 2e1ae52c6..2c500074f 100644 --- a/src/viewer/text/link.c +++ b/src/viewer/text/link.c @@ -1224,7 +1224,7 @@ try_document_key(struct session *ses, struct document_view *doc_view, /* Run through all the links and see if one of them is bound to the * key we test.. */ - i = doc_view->vs->current_link >= 0 ? doc_view->vs->current_link : 0; + i = doc_view->vs->current_link + 1; for (; i < doc_view->document->nlinks; i++) { struct link *link = &doc_view->document->links[i]; @@ -1234,7 +1234,7 @@ try_document_key(struct session *ses, struct document_view *doc_view, return FRAME_EVENT_REFRESH; } } - for (i = 0; i < doc_view->vs->current_link; i++) { + for (i = 0; i <= doc_view->vs->current_link; i++) { struct link *link = &doc_view->document->links[i]; if (key == link->accesskey) { From e347122e6dff75dcb7fc1dc877c77ad16ed13c07 Mon Sep 17 00:00:00 2001 From: Witold Filipczyk Date: Mon, 12 Mar 2007 16:43:17 +0100 Subject: [PATCH 11/11] get_attr_value: do not do trim_chars trim_chars was called only in debug mode and the results of the get_attr_val for value=" something " in debug mode differ from normal and fastmem mode. [ From commit c4500039b2b97564454cc45af0d55bef66b6d350 on the witekfl branch. --KON ] --- src/document/html/parser/parse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/document/html/parser/parse.c b/src/document/html/parser/parse.c index 46e636f8f..135f64599 100644 --- a/src/document/html/parser/parse.c +++ b/src/document/html/parser/parse.c @@ -211,7 +211,7 @@ found_endattr: mem_free(saved_attr); } - set_mem_comment(trim_chars(attr, ' ', NULL), name, strlen(name)); + set_mem_comment(attr, name, strlen(name)); return attr; } else {