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.
Remove enum main_action_offset, enum edit_action_offset, and enum
menu_action_offset. It seems the original plan (in commit
174eabf1a448d3f084a318aab77805828f35c42e on 2005-05-16) was to include
the action flags in the action IDs, perhaps with something like:
ACT_##map##_##action = ACT_##map##_OFFSET_##action | flags
However, this OR operation was never implemented; each ACT_*_*
constant had the same value as the corresponding ACT_*_OFFSET_*,
and the code that looked for flags in action IDs found only zeroes.
Then on 2005-06-10, a separate action.flags member was added, and
the flag checks were corrected to read that instead. So, it seems
safe to say that the original plan has been discarded and the offset
enumerations won't be needed.
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.
In recent ELinks release announcements, I have described:
This release of ELinks is mostly licensed under version 2 of the GNU
General Public License. More permissive licences apply to some parts
of it, and there is also one test file under the GNU Free Documentation
License; please see COPYING for the list.
Remove that test file, so its GPL-incompatible licence need not be
mentioned in future announcements. The file however remains
downloadable as part of the elinks.git repository and releases like
elinks-0.11.6.tar.gz. Those should still be covered by the licence.
After the recent ecmascript_get_interpreter change, I got an assertion
failure in render_document, which calls ecmascript_reset_state and
then asserts that it has set vs->ecmascript != NULL.
ecmascript_reset_state cannot guarantee that because there might not
even be enough free memory for mem_calloc(1, sizeof(struct
ecmascript_interpreter). So, replace the assertion in render_document
with error handling, and likewise in call_onsubmit_and_submit.
This should fix a crash in:
at /home/Kalle/src/elinks-0.12/src/ecmascript/spidermonkey.c:251
at /home/Kalle/src/elinks-0.12/src/ecmascript/ecmascript.c:104
at /home/Kalle/src/elinks-0.12/src/viewer/text/vs.c:64
It seems that spidermonkey_get_interpreter failed and returned NULL to
ecmascript_get_interpreter, which did not check the return value and
behaved as if the ECMAScript interpreter had been properly initialized.
This caused destroy_vs to call ecmascript_put_interpreter, but
backend_data which should have been a JSContext * was NULL, causing
a crash in SpiderMonkey.
An alternative fix might be to make spidermonkey_put_interpreter skip
the JS_DestroyContext call if ctx is NULL. However, I think it is
better to make sure ecmascript_get_interpreter returns NULL if
spidermonkey_get_interpreter fails, so that vs->ecmascript is left
NULL and there's no chance that some other code might try to
dereference the (JSContext *) NULL.
Perhaps because of bug 981, if one opened hundreds of pages with
elinks --remote openURL(...), then ELinks 0.11.4 could crash with a
SIGSEGV in JS_InitClass called from spidermonkey_get_interpreter.
SpiderMonkey ran out of memory and began returning NULL and JS_FALSE
but ELinks didn't notice them and pressed on. Add some checks to
avoid the crash, although the underlying out-of-memory error remains.
The old code failed to write pending spaces before changing the
background color. That seems hard to fix without duplicating code,
and ELinks pads dumped lines to the requested width in these color
modes anyway, so this commit just makes ELinks write all spaces
immediately when colors are being used.
Try the following command before and after this commit:
elinks --no-home --eval "set document.colors.use_document_colors = 2" \
--dump-color-mode 1 --dump test/color.html
In DUMP_FUNCTION_SPECIALIZED, use isscreensafe_ucs (for UTF-8) or
isscreensafe (for unibyte) to detect control characters, and replace
them with spaces. add_document_to_string already did the same.
In DUMP_FUNCTION_SPECIALIZED (used by elinks --dump), detect the
second cell of double-cell (aka fullwidth) characters by comparing to
UCS_NO_CHAR, like add_document_to_string does. Don't use
unicode_to_cell for this any more.
Also, ignore the colors and attributes of the second cell; don't
output any escape sequences for them.
In start_document_refresh, use register_bottom_half instead of
install_timer if the timeout is 0 because install_timer asserts that it is
given a delay greater than 0.
Add a test case, test/refresh-0timeout.html. Note that
document.browse.minimum_refresh_time must be set to 0 to reproduce the
assertion failure.
Add a 'Search contents' button to the cache manager that searches
through the cache items' data rather than their metadata.
Add match_cache_entry_contents.
Add push_cache_hierbox_search_button and
push_cache_hierbox_search_contents_button, which call
push_hierbox_search_button after setting box->ops to either
cache_entry_listbox_ops or cache_entry_listbox_ops_match_contents,
respectively, which define the appropriate match callback for the
hierbox search code.
Add strlcasestr, used in the new match_cache_entry_contents routine.
Add an "Invalidate" button to the cache manager, which is useful when
a cache entry is locked and cannot be deleted but one does not want
ELinks to use that cache entry.
This is especially useful for showing that neither dump_truecolor_utf8
nor dump_truecolor_unibyte modifies its static color[] variable and it
therefore does not matter whether those functions use the same array
or not.
With all the comments and macros needed for this, the source files
don't become much shorter, but anyway I hope they'll be easier to
maintain this way.
This code was included in four variants of dump_to_file().
Move it to a new function dump_references() and make dump_to_file()
then call that. This makes the code size a little smaller.
The time cost will be negligible.