[BUG]: Redirect to previous page + Reload crashes
Dillo crashes as follows: * Point Dillo at http://starurchin.org:8080/dillo/here * Click the "There" link. Dillo is redirected back to the "Here" page. * Click the "Reload" button. * Dillo crashes inside a_Nav_reload() (nav.c) at the line "if (URL_FLAGS(url) & URL_Post) {" because url is NULL. I noticed this a few weeks ago but only just found a simple way to trigger it. Any ideas why this is happening? Regards, Jeremy Henty
Jeremy wrote:
Dillo crashes as follows: * Point Dillo at http://starurchin.org:8080/dillo/here * Click the "There" link. Dillo is redirected back to the "Here" page. * Click the "Reload" button. * Dillo crashes inside a_Nav_reload() (nav.c) at the line "if (URL_FLAGS(url) & URL_Post) {" because url is NULL.
I noticed this a few weeks ago but only just found a simple way to trigger it. Any ideas why this is happening?
The problem went away when I tried the attached change, but I may be treating the symptom rather than the disease. It must not be the result of a very recent change, because I have a dillo from April 8 that crashes doing this.
On Sat, May 24, 2008 at 03:22:10AM +0000, corvid wrote:
Jeremy wrote:
Dillo crashes as follows: * Point Dillo at http://starurchin.org:8080/dillo/here * Click the "There" link. Dillo is redirected back to the "Here" page. * Click the "Reload" button. * Dillo crashes inside a_Nav_reload() (nav.c) at the line "if (URL_FLAGS(url) & URL_Post) {" because url is NULL.
The problem went away when I tried the attached change, but I may be treating the symptom rather than the disease.
diff -pur dillo2/src/nav.c dillo2-cur/src/nav.c --- dillo2/src/nav.c 2008-05-10 20:02:22.000000000 +0000 +++ dillo2-cur/src/nav.c 2008-05-24 03:03:50.000000000 +0000 @@ -185,6 +185,8 @@ static void Nav_stack_clean(BrowserWindo void *data = dList_nth_data (bw->nav_stack, i - 1); dList_remove_fast (bw->nav_stack, data); dFree(data); + if (bw->nav_stack_ptr >= a_Nav_stack_size(bw)) + bw->nav_stack_ptr = a_Nav_stack_size(bw) - 1; } }
I think this addresses the real problem, which is that bw->nav_stack_ptr is an index into bw->nav_stack , so if we modify the latter we must ensure the former is valid. Reviewing the rest of nav.c I think there is a similar potential problem with Nav_stack_insert() , which also modifies bw->nav_stack . Shouldn't it increment bw->nav_stack_ptr if it is after the insertion point? Otherwise the displayed item might change unexpectedly. Regards, Jeremy Henty
On Sat, May 24, 2008 at 08:04:19PM +0100, Jeremy Henty wrote:
Reviewing the rest of nav.c I think there is a similar potential problem with Nav_stack_insert() , which also modifies bw->nav_stack .
On second thoughts, Nav_stack_insert() seems broken. It frees the pointer at the insertion point, so it clearly expects the new item to replace the old. But then it calls dList_insert_pos() which does *not* replace the item at the insertion point, instead it extends the list and moves everything up to make space. So now we have a freed pointer in bw->nav_stack . Either Nav_stack_insert() should not be freeing the pointer or it should call a dList_*() function that replaces the item it has freed. Which? Regards, Jeremy Henty
Jeremy wrote:
On Sat, May 24, 2008 at 08:04:19PM +0100, Jeremy Henty wrote:
Reviewing the rest of nav.c I think there is a similar potential problem with Nav_stack_insert() , which also modifies bw->nav_stack .
On second thoughts, Nav_stack_insert() seems broken. It frees the pointer at the insertion point, so it clearly expects the new item to replace the old. But then it calls dList_insert_pos() which does *not* replace the item at the insertion point, instead it extends the list and moves everything up to make space. So now we have a freed pointer in bw->nav_stack .
Either Nav_stack_insert() should not be freeing the pointer or it should call a dList_*() function that replaces the item it has freed. Which?
Since it looks like Nav_stack_truncate() already got rid of everything at stack_idx and beyond, maybe Nav_stack_insert() could be replaced by a Nav_stack_append() function.
On Sun, May 25, 2008 at 12:31:51AM +0000, corvid wrote:
Since it looks like Nav_stack_truncate() already got rid of everything at stack_idx and beyond, maybe Nav_stack_insert() could be replaced by a Nav_stack_append() function.
Good point! And there is no need for Nav_stack_insert() to update bw->nav_stack_ptr . In fact it *mustn't* do that because the code that calls Nav_stack_insert() assumes that bw->nav_stack_ptr *won't* change and increments it itself. Here's a patch that replaces Nav_stack_insert() by Nav_stack_append() as you suggest. Regards, Jeremy Henty
On Sat, May 24, 2008 at 03:22:10AM +0000, corvid wrote:
Jeremy wrote:
Dillo crashes as follows: * Point Dillo at http://starurchin.org:8080/dillo/here * Click the "There" link. Dillo is redirected back to the "Here" page. * Click the "Reload" button. * Dillo crashes inside a_Nav_reload() (nav.c) at the line "if (URL_FLAGS(url) & URL_Post) {" because url is NULL.
I noticed this a few weeks ago but only just found a simple way to trigger it. Any ideas why this is happening?
The problem went away when I tried the attached change, but I may be treating the symptom rather than the disease.
It must not be the result of a very recent change, because I have a dillo from April 8 that crashes doing this.
diff -pur dillo2/src/nav.c dillo2-cur/src/nav.c --- dillo2/src/nav.c 2008-05-10 20:02:22.000000000 +0000 +++ dillo2-cur/src/nav.c 2008-05-24 03:03:50.000000000 +0000 @@ -185,6 +185,8 @@ static void Nav_stack_clean(BrowserWindo void *data = dList_nth_data (bw->nav_stack, i - 1); dList_remove_fast (bw->nav_stack, data); dFree(data); + if (bw->nav_stack_ptr >= a_Nav_stack_size(bw)) + bw->nav_stack_ptr = a_Nav_stack_size(bw) - 1; } }
Committed. -- Cheers Jorge.-
participants (3)
-
corvid＠lavabit.com -
jcid＠dillo.org -
onepoint＠starurchin.org