Commit ff136e5116 on 2006-07-16 made
lun_resume() check if cdf_hop->data points to struct codw_hop,
and transform that to struct cmdw_hop if so. Move the transform
into a separate function called from continue_download_do().
This way, the structures used with create_download_file() no longer
need to begin with int magic.
To reproduce:
- Configure with --enable-debug.
- Go to http://elinks.cz/
- Set the cursor on the "About" link and press d to download,
- ELinks asks where to save the file. Cancel that with Esc.
This leaves session.download_uri != NULL.
- Go to /etc/passwd
- ELinks asks what to do with the file. Choose to download.
- ELinks asks where to save the file. Type the name of a new file
and press Enter.
- Again go to /etc/passwd
- ELinks asks what to do with the file. Choose to download.
- ELinks asks where to save the file. Type the same name as before
and press Enter.
- ELinks asks whether to resume or overwrite. Choose to resume.
This changes session.download_uri and leaks the original URI.
- Quit ELinks. It reports memory leaks:
0x88936d8:28 @ alloc'd at /home/Kalle/src/elinks-0.12/src/util/hash.c:89
0x88dac00:95 @ alloc'd at /home/Kalle/src/elinks-0.12/src/protocol/uri.c:1551
0x88c33a8:4104 @ alloc'd at /home/Kalle/src/elinks-0.12/src/util/hash.c:41
This commit fixes the leak, but it's still a bug that lun_resume() can
replace the session.download_uri that will be used by another pending
download. In particular, this might happen if the user first presses
d to download, and then while ELinks is asking for the file name, a
web script changes window.location to a different URI and that causes
ELinks to ask what to do with the file. So I'm leaving the FIXME
comment in for now.
If init_file_download() succeeds (returning non-NULL), it saves the
file descriptor to file_download->handle, whence abort_download() will
close it. However, if init_file_download() fails, the caller is
responsible of closing the file, something common_download_do() and
continue_download_do() failed to do. There was no problem with
bittorrent_download() because that uses -1 as the fd.
If init_file_download() succeeds (returning non-NULL), it saves the
file-name pointer to file_download->file, whence abort_download() will
free it. However, if init_file_download() fails, the caller is
responsible of freeing the name. bittorrent_download() already did so
but common_download_do() and continue_download_do() didn't.
Doxygen isn't too good at documenting the parameters of a callback
within the documentation of a parameter that points to the callback.
A typedef provides a better place to document the parameters.
If the user chose File -> Save formatted document and typed the name
of an existing file, ELinks offered to resume downloading the file.
There are a few problems with that:
* save_formatted_finish does not actually support resuming. It would
instead overwrite the beginning of the file and not truncate it.
* When save_formatted calls create_download_file, cdf_hop->data
ends up pointing to struct document. If the user then chooses to
resume, lun_resume would read *(int *)cdf_hop->data, hoping to
get cmdw_hop.magic or codw_hop.magic. struct document does not
begin with any such magic value.
* Because ELinks already has the formatted document in memory,
resuming saves neither time nor I/O.
So don't show the "Resume download of the original file" button in
this situation.
Yet another valiant wack at the beast. This one violates abstractions
a little less deeply, so maybe it will work better.
The last attempt caused a crash when a tab was cloned after the tab's
loading had been aborted.
(cherry picked from commit 76377d9714)
Kalle reported that after commit 5c96d430c9,
ELinks would crash if the document in the old tab was still loading when a
new tab was opened. The problem was that the new session's download.data
pointer was not updated to point to the session as doc_loading_callback
expects.
Instead of just calling render_document_frames, set up the download and
call load_uri.
(cherry picked from commit d6116ca83a)
In setup_session, use copy_location, add_to_history, and
render_document_frames instead of goto_uri and copy_vs to copy the base
tab's view state. By avoiding goto_uri, setup_session now bypasses MIME
checks, form post confirmations, malicious URL checks, and so on when
copying the base tab's current location and view state to the new tab,
so the new tab should get exactly what was loaded in the base tab.
This fixes bug 765: Opening a new tab can ask about the document of the
previous tab.
(cherry picked from commit 5c96d430c9)
Conflicts:
src/session/session.c:
Both elinks-0.12 and master had the ses->doc_view->vs
= vs assignment, but only elinks-0.12 had vs->doc_view
= ses->doc_view as well. Also, struct connection_state
had been added after the original patch.
In the task.c line 517 there is:
if (is_in_progress_state((*download_p)->state)) {
if (have_location(ses))
*download_p = &cur_loc(ses)->download;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Here the download was changed. download->data and download->callback
were NULL after the assignment, but later in loading_callback
only download->callback had new value. download->data was still NULL.
Yet another valiant wack at the beast. This one violates abstractions
a little less deeply, so maybe it will work better.
The last attempt caused a crash when a tab was cloned after the tab's
loading had been aborted.
Kalle reported that after commit 5c96d430c9,
ELinks would crash if the document in the old tab was still loading when a
new tab was opened. The problem was that the new session's download.data
pointer was not updated to point to the session as doc_loading_callback
expects.
Instead of just calling render_document_frames, set up the download and
call load_uri.
At the end of the destroy_vs there two assignments vs->doc_view->vs = NULL and
vs->doc_view = NULL. In the setup_session the copy_vs left the vs "unbound"
with any variable. At least one of these two is wrong.
C99 6.7.4p3 and 6.7.4p6 set some constraints on what can be done in
inline functions and how they can be declared. In particular, any
function declared inline must also be defined in the same translation
unit. To comply with that, remove inline specifiers from function
declarations in header files when the functions are not also defined
in those header files.
Sun Studio 11 on Solaris 9 is stricter than C99 and does not allow
references to static identifiers in extern inline functions. Make the
configure script detect this and define NONSTATIC_INLINE accordingly
in config.h. Then use that in the definitions of all non-static
inline functions.
Document the restrictions and this scheme in doc/hacking.txt.
In src/session/task.c, if ses_goto() was going to ask the user to
confirm, it did:
task->session_task.target.frame = null_or_stracpy(target_frame);
It added the struct task to a memory_list, so the structure was freed
when the message box was closed. The target frame string was however
never freed. To fix this leak, add the target frame string to the
memory_list too.
Alternatively, this could have been fixed by making post_yes() and
post_no() free the string. It is however a bit better to use the
memory_list because msg_box() frees that even if it is unable to
display the message box.
These functions now expect or return strings in UTF-8:
delete_folder_by_name (sneak in a const, too), bookmark_terminal_tabs,
open_bookmark_folder, and get_auto_save_bookmark_foldername_utf8 (new
function).
Change test/imgmap2.html so it can be used for testing this too.
Debian Iceweasel 3.0.4 does not appear to support such external
client-side image maps. Well, that's one place where ELinks is
superior, I guess. There might be a security problem though if ELinks
were to let scripts of the referring page examine the links in the
image map.
This simplifies the callers a little and may help implement
simultaneous support for different charsets on different terminals
of the same type (bug 1064).
Except if they have external handlers.
When ELinks receives an event from a terminal, move that terminal to
the beginning of the global "terminals" list, so that the terminals
are always sorted according to the time of the most recent use. Note,
this affects the numbering of bookmark folders in session snapshots.
Add get_default_terminal(), which returns the most recently used
terminal that is still open. Use that in various places that
previously used terminals.prev or terminals.next. Four functions
fetch the size of the terminal for User-Agent headers, and
get_default_terminal() is not really right, but neither was the
original code; add TODO comments in those functions.
When the user chooses "Background and Notify", associate the download
with the terminal where the dialog box is. So any later messages will
then appear in that terminal, if it is still open. However, don't
change the terminal if the download has an external handler.
When a download gets some data, don't immediately check the associated
terminal. Instead, wait for the download to end. Then, if the
terminal of the download has been closed, use get_default_terminal()
instead. If there is no default terminal either, just skip any
message boxes.
ELinks attempted to display a message box on file_download.term, but
it had already closed that terminal and freed the struct terminal. To
fix this, reset file_download.term pointers to NULL when the terminal
is about to be destroyed. Also, assert in download_data_store() that
file_download.term is either NULL or in the global "terminals" list.
Reported by أحمد المحمودي.
(cherry picked from commit 6e2476ea4d)
Replace almost all uses of enum connection_state with struct
connection_status. This removes the assumption that errno values used
by the system are between 0 and 100000. The GNU Hurd uses values like
ENOENT = 0x40000002 and EMIG_SERVER_DIED = -308.
This commit is derived from my attachments 450 and 467 to bug 1013.
Conflicts:
NEWS
configure.in
The following files also conflicted, but they had not been manually
edited in the elinks-0.12 branch after the previous merge, so I just
kept the 0.13.GIT versions:
doc/man/man1/elinks.1.in
doc/man/man5/elinks.conf.5
doc/man/man5/elinkskeys.5
po/fr.po
po/pl.po
I am reverting all copiousoutput support because of bug 917.
This reverts commit f6115e65ec.
Conflicts:
src/session/download.h: type_query.cgi, and Doxygen comments.
I am reverting all copiousoutput support because of bug 917.
This reverts commit a2c12d7653.
Conflicts:
src/session/download.c: The int_min vs. int_max change had
already been obsoleted by using safe_strncpy instead,
in commit efcd6c9758 for bug 896 on 2007-07-24.
Also, TERM_EXEC_FG and TERM_EXEC_BG had been added.
I am reverting all copiousoutput support because of bug 917.
This reverts commit 6ead4e9c65.
Conflicts:
src/session/download.c: TERM_EXEC_FG and TERM_EXEC_BG had been
added after the original commit.
I am reverting all copiousoutput support because of bug 917.
This reverts commit 4dc4ea47f2.
Conflicts:
src/network/connection.h: After the original commit, the declaration
of copiousoutput_data had been changed to use the LIST_OF macro.
Also, connection.cgi had been added next to the connection.popen
member added by the original commit.
src/session/download.c: After the original commit, the definition of
copiousoutput_data had been changed to use the INIT_LIST_OF macro.
If the user opens the same file again after it is in the cache, then
ELinks does not always open a new connection, so download->conn can be
NULL in init_type_query(), and download->conn->cgi would crash.
Don't read that, then; instead add a new flag cache_entry.cgi, which
http_got_header() sets or clears as soon as possible after the cache
entry has been created.
(cherry picked from commit 81f8ee1fa2)
CGI scripts are distinguishable from normal files. I hope that this
fixes the bug 991. This commit also reverts the previous revert.
(cherry picked from commit 7ceba1e461)
In setup_session, use copy_location, add_to_history, and
render_document_frames instead of goto_uri and copy_vs to copy the base
tab's view state. By avoiding goto_uri, setup_session now bypasses MIME
checks, form post confirmations, malicious URL checks, and so on when
copying the base tab's current location and view state to the new tab,
so the new tab should get exactly what was loaded in the base tab.
This fixes bug 765: Opening a new tab can ask about the document of the
previous tab.
Previously, init_type_query would check to make sure that it doesn't create a
duplicate type query and would return NULL if it otherwise would create a
duplicate. Then setup_download_handler would return 0 to do_move.
This patch changes setup_download_handler to return 1 to do_move in this
situation so that do_move stops trying to load the document.
This avoid a crash when loading twice the same file in the same tab when
loading the file opens a type query.
If the user opens the same file again after it is in the cache, then
ELinks does not always open a new connection, so download->conn can be
NULL in init_type_query(), and download->conn->cgi would crash.
Don't read that, then; instead add a new flag cache_entry.cgi, which
http_got_header() sets or clears as soon as possible after the cache
entry has been created.
CGI scripts are distinguishable from normal files. I hope that this
fixes the bug 991. This commit also reverts the previous revert.
(cherry picked from commit 7ceba1e461)
This reverts commit 7ceba1e461,
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.
This reverts commit d0be89a16c, 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.
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.)
After copying the base session's viewstate, restore the ses->doc_view->vs
pointer.
This follows up on commit 65321923b9:
In setup_session, copy the viewstate for the new session from the base
session.
This fixes bug 977: When opening a javascript: link in a new tab, an
assertion on ses->doc_view->vs in ecmascript_protocol_handler would fail.
In init_remote_session, pass open_uri_in_new_tab a false argument for the based parameter so that the new tab does not get the old tabs current location in its history.
start_document_refreshes() performs the NULL-pointer checks that
previously all callers to start_document_refresh() must perform
and then calls start_document_refresh().
start_document_refreshes performs the NULL-pointer checks that previously all callers to start_document_refresh must perform and then calls start_document_refresh.
Pass the session with some get_opt_* calls. These are the low-hanging fruit. Some places will be difficult because we don't have the session or for other reasons.
This cleans up and changes the calling convention of
load_additional_file(), so that it now grab its own doc_view using
current_frame() and the struct session pointer in the file_to_load
object. By always looking up the current frame, corruption of doc_view
due to rerendering of the upper frame document is avoided.
The fix extends commit 6afdbf608f by
Witold, which moved the getting of doc_view in the other caller of
load_additional_file(). The additional clean up ensures that any future
users of load_additional_file() will not reintroduce a similar bug.
A possible future optimization would be to change load_additional_file()
to take the referrer as an argument.
Use it for the actual I/O only. Previously, defining CONFIG_UTF8 and
enabling UTF-8 used to force many strings to the UTF-8 charset
regardless of the terminal charset option. Now, those strings always
follow the terminal charset. This fixes bug 914 which was caused
because _() returned strings in the terminal charset and functions
then assumed they were in UTF-8. This reduction in the effects of
UTF-8 I/O may also simplify future testing.
The code works both with copiousoutput and without it.
[ Part 1/2 of commit c25c41bd18 on the
witekfl branch. I'm leaving out the part that depends on commit
469481b272, which is not yet safe to
apply. --KON ]
the handler reads data from stdin. I think it only works with copiousoutput.
[ Part 1/2 of commit 4a7b9415e1 on the
witekfl branch, fixes bug 916. I'm leaving out delayed_goto_uri()
for now because I don't understand its purpose. --KON ]
Don't cast function pointers; calling functions via pointers of
incorrect types is not guaranteed to work. Instead, define the
functions with the desired types, and make them cast the incoming
parameters. Or define wrapper functions if the return types don't
match.
really_exit_prog wasn't being used outside src/dialogs/menu.c,
and I had to change its parameter type, so it's now static.
