From 7bdeb3188e691c3944637cc8f0085d91b94519e5 Mon Sep 17 00:00:00 2001 From: Jonas Fonseca Date: Sat, 8 Mar 2008 20:15:19 +0100 Subject: [PATCH 1/9] When releasing po files should also be updated --- doc/release.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/release.txt b/doc/release.txt index 8eb76f5a..f51f9b9c 100644 --- a/doc/release.txt +++ b/doc/release.txt @@ -15,6 +15,7 @@ When releasing a new version - Change VERSION in the top of configure.in to hold the new version number. - Update the manpages so the will have the new release number by first building the source, followed by making the `update-man' target in doc/. + - Update po files by running `make update-po` in po/`. - Commit these changes. - Create a signed tag having the version ("ELinks X.X.X") as the subject and using the changelog create above as the body. Use something like: From a6966e94725cda2eb9e437e42bccb561682f2236 Mon Sep 17 00:00:00 2001 From: Witold Filipczyk Date: Fri, 7 Mar 2008 19:56:15 +0100 Subject: [PATCH 2/9] Bug 991: Replace '%s' by % in the mailcap.c Currently, when ELinks passes the name of a local file to an external MIME handler program, it encodes the name as a URI. Programs typically do not expect this, and they then fail to open the file. ELinks should instead quote the file name for the shell. Unfortunately, Debian has lines like this in /etc/mailcap: audio/mpeg; xmms '%s'; test=test "$DISPLAY" != "" If ELinks were changed to replace the %s with e.g. '/home/Kalle/doc/Topfield/How to upgraded the Firmware(English).pdf' (quotes included), then the quotes would cancel out and the shell would split the file name into multiple arguments. That could even provide a way for malicious persons to make ELinks run arbitrary shell commands. The examples in RFC 1524 all have %s without any quotes. Debian has two bug reports about the quoting behaviour: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=90483 http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=221717 This patch therefore tries to detect whether the %s has been quoted already, and remove the quotes if so. That way, the next patch will be able to safely add its own quotes. This removal of quotes applies only to mailcap files; MIME handlers defined in elinks.conf should already be in the format preferred by ELinks. (The patch was attachment 438 of bug 991, by Witold Filipczyk. This commit message was written by Kalle Olavi Niemitalo.) --- src/mime/backend/mailcap.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/mime/backend/mailcap.c b/src/mime/backend/mailcap.c index 752c626c..76ea78b9 100644 --- a/src/mime/backend/mailcap.c +++ b/src/mime/backend/mailcap.c @@ -498,13 +498,24 @@ format_command(unsigned char *command, unsigned char *type, int copiousoutput) while (*command) { unsigned char *start = command; - while (*command && *command != '%' && *command != '\\') + while (*command && *command != '%' && *command != '\\' && *command != '\'') command++; if (start < command) add_bytes_to_string(&cmd, start, command - start); - if (*command == '%') { + switch (*command) { + case '\'': /* Debian's '%s' */ + command++; + if (!strncmp(command, "%s'", 3)) { + command += 3; + add_char_to_string(&cmd, '%'); + } else { + add_char_to_string(&cmd, '\''); + } + break; + + case '%': command++; if (!*command) { done_string(&cmd); @@ -522,13 +533,16 @@ format_command(unsigned char *command, unsigned char *type, int copiousoutput) add_to_string(&cmd, type); } command++; + break; - } else if (*command == '\\') { + case '\\': command++; if (*command) { add_char_to_string(&cmd, *command); command++; } + default: + break; } } #if 0 From 19079733dfc9d5bfa6d153cb6e19cd0c6b80b9da Mon Sep 17 00:00:00 2001 From: Witold Filipczyk Date: Tue, 8 Jan 2008 22:00:19 +0100 Subject: [PATCH 3/9] Bug 991: URI-decode and shell-quote file name for MIME handler Currently, when ELinks passes the name of a local file to an external MIME handler program, it encodes the name as a URI. Programs typically do not expect this, and they then fail to open the file. This patch makes ELinks instead quote the file name for the shell. (The patch was attachment 425 of bug 991, by Witold Filipczyk. This commit message was written by Kalle Olavi Niemitalo.) --- src/session/download.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/session/download.c b/src/session/download.c index 56d58c8a..10d7eaa9 100644 --- a/src/session/download.c +++ b/src/session/download.c @@ -812,7 +812,7 @@ subst_file(unsigned char *prog, unsigned char *file) cygwin_conv_to_full_win32_path(file, new_path); add_to_string(&name, new_path); #else - add_to_string(&name, file); + add_shell_quoted_to_string(&name, file, strlen(file)); #endif prog++; } @@ -1093,6 +1093,7 @@ tp_open(struct type_query *type_query) unsigned char *handler = NULL; if (file) { + decode_uri(file); handler = subst_file(type_query->external_handler, file); mem_free(file); } From 5419414b5965a168107a0d3f791941b46b26ccfa Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sat, 8 Mar 2008 20:39:10 +0200 Subject: [PATCH 4/9] Bug 991: Tell users not to quote % in MIME handlers. ELinks quotes the file name automatically and user-written quote characters would just interfere with that. --- src/mime/backend/default.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mime/backend/default.c b/src/mime/backend/default.c index 30f27319..8570cdcc 100644 --- a/src/mime/backend/default.c +++ b/src/mime/backend/default.c @@ -72,7 +72,8 @@ static struct option_info default_mime_options[] = { "program", 0, "", /* xgettext:no-c-format */ N_("External viewer for this file type. '%' in this string will be\n" - "substituted by a file name.")), + "substituted by a file name.\n" + "Do _not_ put single- or double-quotes around the % sign.")), INIT_OPT_TREE("mime", N_("File extension associations"), From 13a04b8fbc6a839ded061fd3134a847e1c2fc6d5 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sat, 8 Mar 2008 20:09:59 +0200 Subject: [PATCH 5/9] Bug 991: Revert "When requested to open local files with a handler use the file in place" This reverts commit d0be89a16c13d0b228c9dae40bd6cb4137f75b57, and thus restores the ELinks 0.11 behaviour: always copy the data to a temporary file before passing it to a MIME handler, even if the "file:" URI scheme is being used. Previously, ELinks 0.12.GIT passed the name of the original file directly to the handler. That was more efficient but unfortunately gave the wrong result with local CGI. The commit being reverted also claims to partially fix bug 238 (caching of local files). That bug is still open. --- src/session/download.c | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/src/session/download.c b/src/session/download.c index 10d7eaa9..a04e82fc 100644 --- a/src/session/download.c +++ b/src/session/download.c @@ -1088,30 +1088,6 @@ tp_open(struct type_query *type_query) return; } - if (type_query->uri->protocol == PROTOCOL_FILE) { - unsigned char *file = get_uri_string(type_query->uri, URI_PATH); - unsigned char *handler = NULL; - - if (file) { - decode_uri(file); - handler = subst_file(type_query->external_handler, file); - mem_free(file); - } - - if (handler) { - if (type_query->copiousoutput) - read_from_popen(type_query->ses, handler, NULL); - else - exec_on_terminal(type_query->ses->tab->term, - handler, "", - type_query->block ? TERM_EXEC_FG : TERM_EXEC_BG); - mem_free(handler); - } - - done_type_query(type_query); - return; - } - continue_download(type_query, ""); } From 7ceba1e46131a96ee78ce999dfbe14e359d8cbcb Mon Sep 17 00:00:00 2001 From: Witold Filipczyk Date: Sun, 9 Mar 2008 11:50:09 +0000 Subject: [PATCH 6/9] bug 991: Added the bit field cgi to the structs connection and type_query. CGI scripts are distinguishable from normal files. I hope that this fixes the bug 991. This commit also reverts the previous revert. --- src/network/connection.h | 1 + src/protocol/file/cgi.c | 1 + src/session/download.c | 28 ++++++++++++++++++++++++++++ src/session/download.h | 1 + 4 files changed, 31 insertions(+) diff --git a/src/network/connection.h b/src/network/connection.h index 61931e19..311bbaaf 100644 --- a/src/network/connection.h +++ b/src/network/connection.h @@ -58,6 +58,7 @@ struct connection { unsigned int unrestartable:1; unsigned int detached:1; unsigned int popen:1; + unsigned int cgi:1; /* Each document is downloaded with some priority. When downloading a * document, the existing connections are checked to see if a diff --git a/src/protocol/file/cgi.c b/src/protocol/file/cgi.c index 75830fb7..8cb02bb9 100644 --- a/src/protocol/file/cgi.c +++ b/src/protocol/file/cgi.c @@ -393,6 +393,7 @@ execute_cgi(struct connection *conn) /* Use data socket for passing the pipe. It will be cleaned up in * close_pipe_and_read(). */ conn->data_socket->fd = pipe_write[1]; + conn->cgi = 1; set_nonblocking_fd(conn->socket->fd); set_nonblocking_fd(conn->data_socket->fd); diff --git a/src/session/download.c b/src/session/download.c index a04e82fc..45f99964 100644 --- a/src/session/download.c +++ b/src/session/download.c @@ -977,6 +977,9 @@ init_type_query(struct session *ses, struct download *download, { struct type_query *type_query; + assert(download && download->conn); + if_assert_failed return NULL; + /* There can be only one ... */ foreach (type_query, ses->type_queries) if (compare_uri(type_query->uri, ses->loading_uri, 0)) @@ -990,6 +993,7 @@ init_type_query(struct session *ses, struct download *download, type_query->target_frame = null_or_stracpy(ses->task.target.frame); type_query->cached = cached; + type_query->cgi = download->conn->cgi; object_lock(type_query->cached); move_download(download, &type_query->download, PRI_MAIN); @@ -1088,6 +1092,30 @@ tp_open(struct type_query *type_query) return; } + if (type_query->uri->protocol == PROTOCOL_FILE && !type_query->cgi) { + unsigned char *file = get_uri_string(type_query->uri, URI_PATH); + unsigned char *handler = NULL; + + if (file) { + decode_uri(file); + handler = subst_file(type_query->external_handler, file); + mem_free(file); + } + + if (handler) { + if (type_query->copiousoutput) + read_from_popen(type_query->ses, handler, NULL); + else + exec_on_terminal(type_query->ses->tab->term, + handler, "", + type_query->block ? TERM_EXEC_FG : TERM_EXEC_BG); + mem_free(handler); + } + + done_type_query(type_query); + return; + } + continue_download(type_query, ""); } diff --git a/src/session/download.h b/src/session/download.h index 79923d2e..39fb2904 100644 --- a/src/session/download.h +++ b/src/session/download.h @@ -49,6 +49,7 @@ struct type_query { unsigned char *external_handler; int block; unsigned int copiousoutput:1; + unsigned int cgi:1; /* int frame; */ }; From cd4a9d77b953f05c25c21a7e7ffd76e912a54aea Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Tue, 11 Mar 2008 10:41:45 +0200 Subject: [PATCH 7/9] Revert "bug 991: Added the bit field cgi to the structs connection and type_query." This reverts commit 7ceba1e46131a96ee78ce999dfbe14e359d8cbcb, which is causing an assertion to fail if I open the same PDF twice in a row, even if I cancel the dialog box when ELinks first asks which program to run: INTERNAL ERROR at /home/Kalle/src/elinks-0.12/src/session/download.c:980: assertion download && download->conn failed! Forcing core dump! Man the Lifeboats! Women and children first! But please DO NOT report this as a segfault!!! It is an internal error, not a normal segfault, there is a huge difference in these for us the developers. Also, noting the EXACT error you got above is crucial for hunting the problem down. Thanks, and please get in touch with us. Program received signal SIGSEGV, Segmentation fault. [Switching to Thread -1216698688 (LWP 17877)] 0xb7a02d76 in raise () from /lib/libc.so.6 (gdb) backtrace 6 at /home/Kalle/src/elinks-0.12/src/util/error.c:179 fmt=0x816984c "assertion download && download->conn failed!") at /home/Kalle/src/elinks-0.12/src/util/error.c:122 cached=0x8253ca8) at /home/Kalle/src/elinks-0.12/src/session/download.c:980 cached=0x8253ca8, frame=0) at /home/Kalle/src/elinks-0.12/src/session/download.c:1339 at /home/Kalle/src/elinks-0.12/src/session/task.c:493 (More stack frames follow...) There is a fix available but I don't trust it yet. --- src/network/connection.h | 1 - src/protocol/file/cgi.c | 1 - src/session/download.c | 28 ---------------------------- src/session/download.h | 1 - 4 files changed, 31 deletions(-) diff --git a/src/network/connection.h b/src/network/connection.h index 311bbaaf..61931e19 100644 --- a/src/network/connection.h +++ b/src/network/connection.h @@ -58,7 +58,6 @@ struct connection { unsigned int unrestartable:1; unsigned int detached:1; unsigned int popen:1; - unsigned int cgi:1; /* Each document is downloaded with some priority. When downloading a * document, the existing connections are checked to see if a diff --git a/src/protocol/file/cgi.c b/src/protocol/file/cgi.c index 8cb02bb9..75830fb7 100644 --- a/src/protocol/file/cgi.c +++ b/src/protocol/file/cgi.c @@ -393,7 +393,6 @@ execute_cgi(struct connection *conn) /* Use data socket for passing the pipe. It will be cleaned up in * close_pipe_and_read(). */ conn->data_socket->fd = pipe_write[1]; - conn->cgi = 1; set_nonblocking_fd(conn->socket->fd); set_nonblocking_fd(conn->data_socket->fd); diff --git a/src/session/download.c b/src/session/download.c index 45f99964..a04e82fc 100644 --- a/src/session/download.c +++ b/src/session/download.c @@ -977,9 +977,6 @@ init_type_query(struct session *ses, struct download *download, { struct type_query *type_query; - assert(download && download->conn); - if_assert_failed return NULL; - /* There can be only one ... */ foreach (type_query, ses->type_queries) if (compare_uri(type_query->uri, ses->loading_uri, 0)) @@ -993,7 +990,6 @@ init_type_query(struct session *ses, struct download *download, type_query->target_frame = null_or_stracpy(ses->task.target.frame); type_query->cached = cached; - type_query->cgi = download->conn->cgi; object_lock(type_query->cached); move_download(download, &type_query->download, PRI_MAIN); @@ -1092,30 +1088,6 @@ tp_open(struct type_query *type_query) return; } - if (type_query->uri->protocol == PROTOCOL_FILE && !type_query->cgi) { - unsigned char *file = get_uri_string(type_query->uri, URI_PATH); - unsigned char *handler = NULL; - - if (file) { - decode_uri(file); - handler = subst_file(type_query->external_handler, file); - mem_free(file); - } - - if (handler) { - if (type_query->copiousoutput) - read_from_popen(type_query->ses, handler, NULL); - else - exec_on_terminal(type_query->ses->tab->term, - handler, "", - type_query->block ? TERM_EXEC_FG : TERM_EXEC_BG); - mem_free(handler); - } - - done_type_query(type_query); - return; - } - continue_download(type_query, ""); } diff --git a/src/session/download.h b/src/session/download.h index 39fb2904..79923d2e 100644 --- a/src/session/download.h +++ b/src/session/download.h @@ -49,7 +49,6 @@ struct type_query { unsigned char *external_handler; int block; unsigned int copiousoutput:1; - unsigned int cgi:1; /* int frame; */ }; From 9bd5b23ee5b36cb150a42de5d2de9e5be7a8626b Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Tue, 11 Mar 2008 10:54:54 +0200 Subject: [PATCH 8/9] Doxyfile.in: Use @VERSION@. doc/release.txt need not be changed now, because it already did not mention updating the version number in Doxyfile.in. --- doc/Doxyfile.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/Doxyfile.in b/doc/Doxyfile.in index 5f59145c..480a1f9e 100644 --- a/doc/Doxyfile.in +++ b/doc/Doxyfile.in @@ -6,7 +6,7 @@ DOXYFILE_ENCODING = UTF-8 PROJECT_NAME = ELinks -PROJECT_NUMBER = 0.12.GIT +PROJECT_NUMBER = @VERSION@ OUTPUT_DIRECTORY = api CREATE_SUBDIRS = NO OUTPUT_LANGUAGE = English From dafb726a49251a8d006fe46687d8d4f3cf082f97 Mon Sep 17 00:00:00 2001 From: Witold Filipczyk Date: Fri, 14 Mar 2008 16:54:04 +0100 Subject: [PATCH 9/9] bug 976: do not use stdout and stderr in a child processing smb:// libsmbclient's stdout and stderr interferred with ELinks's stdout and stdin. That caused an assertion failure. Now the ELinks uses different streams for processing of the smb protocol. --- src/protocol/smb/smb2.c | 57 +++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 14 deletions(-) diff --git a/src/protocol/smb/smb2.c b/src/protocol/smb/smb2.c index 0a839dbf..4ecd99c5 100644 --- a/src/protocol/smb/smb2.c +++ b/src/protocol/smb/smb2.c @@ -62,6 +62,8 @@ struct module smb_protocol_module = struct_module( /* done: */ NULL ); +static FILE *header_out, *data_out; + /* The child process generally does not bother to free the memory it * allocates. When the process exits, the operating system will free * the memory anyway. There is no point in changing this, because the @@ -71,8 +73,8 @@ struct module smb_protocol_module = struct_module( static void smb_error(int error) { - fprintf(stderr, "text/x-error"); - printf("%d\n", error); + fputs("text/x-error", header_out); + fprintf(data_out, "%d\n", error); exit(1); } @@ -168,7 +170,8 @@ display_entry(const struct smbc_dirent *entry, const unsigned char dircolor[]) /* unknown type */ break; } - puts(string.source); + fputs(string.source, data_out); + fputc('\n', data_out); done_string(&string); } @@ -253,10 +256,11 @@ smb_directory(int dir, struct uri *uri) smb_error(-S_OUT_OF_MEM); } - fprintf(stderr, "text/html"); - fclose(stderr); + fputs("text/html", header_out); + fclose(header_out); - puts(buf.source); + fputs(buf.source, data_out); + fputc('\n', data_out); if (get_opt_bool("document.browse.links.color_dirs")) { color_to_string(get_opt_color("document.colors.dirs"), @@ -264,7 +268,7 @@ smb_directory(int dir, struct uri *uri) } sort_and_display_entries(dir, dircolor); - puts("
"); + fputs("
\n", data_out); smbc_closedir(dir); exit(0); } @@ -329,7 +333,7 @@ do_smb(struct connection *conn) const int errno_from_opendir = errno; char buf[READ_SIZE]; struct stat sb; - int r, res; + int r, res, fdout; int file = smbc_open(url, O_RDONLY, 0); if (file < 0) { @@ -349,12 +353,13 @@ do_smb(struct connection *conn) smb_error(res); } /* filesize */ - fprintf(stderr, "%" OFF_PRINT_FORMAT, + fprintf(header_out, "%" OFF_PRINT_FORMAT, (off_print_T) sb.st_size); - fclose(stderr); + fclose(header_out); + fdout = fileno(data_out); while ((r = smbc_read(file, buf, READ_SIZE)) > 0) { - if (safe_write(STDOUT_FILENO, buf, r) <= 0) + if (safe_write(fdout, buf, r) <= 0) break; } smbc_close(file); @@ -498,6 +503,24 @@ smb_got_header(struct socket *socket, struct read_buffer *rb) } } +static void +close_all_fds_but_two(int header, int data) +{ + int n; + int max = 1024; +#ifdef RLIMIT_NOFILE + struct rlimit lim; + + if (!getrlimit(RLIMIT_NOFILE, &lim)) + max = lim.rlim_max; +#endif + for (n = 3; n < max; n++) { + if (n != header && n != data) close(n); + } +} + + + void smb_protocol_handler(struct connection *conn) { @@ -532,9 +555,14 @@ smb_protocol_handler(struct connection *conn) } if (!cpid) { - dup2(smb_pipe[1], 1); dup2(open("/dev/null", O_RDONLY), 0); - dup2(header_pipe[1], 2); + close(1); + close(2); + data_out = fdopen(smb_pipe[1], "w"); + header_out = fdopen(header_pipe[1], "w"); + + if (!data_out || !header_out) exit(1); + close(smb_pipe[0]); close(header_pipe[0]); @@ -549,7 +577,8 @@ smb_protocol_handler(struct connection *conn) * before exit(), then stdio may attempt to write the * buffers to the wrong files. This might happen for * example if libsmbclient calls syslog(). */ - close_all_non_term_fd(); + + close_all_fds_but_two(smb_pipe[1], header_pipe[1]); do_smb(conn); } else {