Serious Memory Allocation Strategy Problem in Cache_parse_header()
I'm currently in the process of modifying dillo for use in browsing an http media server (yeah, I know it's httpd abuse, but it's only for a local network and I was running Apache anyhow). At present, this basically involves sending a modified downloads plugin an "execute" command and the MIME type when AbortEntry is true in Cache_process_queue() (the plugin then executes an agent which deals with the URL entirely independent of dillo). (Actually, this is functionality that would be nice in the main branch, albeit with its own plugin and user-selectivity. If there's interest, please let me know.) So far, so good -- it seems to work passably well and it only took a couple of hours. Easy peasey. Unfortunately, the first time I click on a 700M video stream, things go horribly wrong. Seems that Cache_parse_header() attempts to allocate memory for the entire content *before* the header has been checked for viewability (since the dispatcher isn't called until Cache_process_queue()). As it happens, this has been reported as bug 726. The upshot of all this is that when you click on a link, there had better be room for the whole thing in heap even though the content may never be read. Otherwise, glib just gives up and dumps core via g_malloc(). I think I can get around this by doing a Content-Type check in Cache_parse_header(). I'd be happy to provide a patch provided I can come up with a reasonably elegant solution. Any advice would be appreciated.
Hi, On Wed, Sep 06, 2006 at 08:12:42PM -0500, R.L. Horn wrote:
I'm currently in the process of modifying dillo for use in browsing an http media server (yeah, I know it's httpd abuse, but it's only for a local network and I was running Apache anyhow). At present, this basically involves sending a modified downloads plugin an "execute" command and the MIME type when AbortEntry is true in Cache_process_queue() (the plugin then executes an agent which deals with the URL entirely independent of dillo).
(Actually, this is functionality that would be nice in the main branch, albeit with its own plugin and user-selectivity. If there's interest, please let me know.)
Yes, more or less this is the plan.
So far, so good -- it seems to work passably well and it only took a couple of hours. Easy peasey. Unfortunately, the first time I click on a 700M video stream, things go horribly wrong.
Seems that Cache_parse_header() attempts to allocate memory for the entire content *before* the header has been checked for viewability (since the dispatcher isn't called until Cache_process_queue()). As it happens, this has been reported as bug 726.
Yes. I fixed this sometime ago in the FLTK2 branch...
The upshot of all this is that when you click on a link, there had better be room for the whole thing in heap even though the content may never be read. Otherwise, glib just gives up and dumps core via g_malloc().
I think I can get around this by doing a Content-Type check in Cache_parse_header(). I'd be happy to provide a patch provided I can come up with a reasonably elegant solution.
Any advice would be appreciated.
OK, a good workaround is to check for huge filesizes while parsing the header and to set as if no Content-length was given in that case. This is simple and will work. (My FLTK2 source uses automatic buffers from a new library I made that also replaces glib). -- Cheers Jorge.-
On Thu, 7 Sep 2006, Jorge Arellano Cid wrote:
(Actually, this is functionality that would be nice in the main branch, albeit with its own plugin and user-selectivity. If there's interest, please let me know.)
Yes, more or less this is the plan.
Great!
OK, a good workaround is to check for huge filesizes while parsing the header and to set as if no Content-length was given in that case. This is simple and will work.
That's kind of what I did. Actually, I just removed the code that had to do with static buffers entirely (while still allowing for a Content-length check). If the GTK branch weren't pretty well deprecated, I'd go ahead and do it right: Add a boolean to the "entry" structure to indicate a fully allocated buffer and complete the allocation (where reasonable) at dispatch time.
On Thu, Sep 07, 2006 at 07:38:27PM -0500, R.L. Horn wrote:
On Thu, 7 Sep 2006, Jorge Arellano Cid wrote:
(Actually, this is functionality that would be nice in the main branch, albeit with its own plugin and user-selectivity. If there's interest, please let me know.)
Yes, more or less this is the plan.
Great!
OK, a good workaround is to check for huge filesizes while parsing the header and to set as if no Content-length was given in that case. This is simple and will work.
That's kind of what I did. Actually, I just removed the code that had to do with static buffers entirely (while still allowing for a Content-length check).
If the GTK branch weren't pretty well deprecated, I'd go ahead and do it right: Add a boolean to the "entry" structure to indicate a fully allocated buffer and complete the allocation (where reasonable) at dispatch time.
The flag is already there:
if ( (Length = Cache_parse_field(header, "Content-Length")) != NULL ) { entry->Flags |= CA_GotLength; /*** Here! ***/ entry->TotalSize = strtol(Length, NULL, 10); g_free(Length); if (entry->TotalSize < entry->ValidSize) entry->TotalSize = 0; }
-- Cheers Jorge.-
participants (2)
-
Jorge Arellano Cid -
R.L. Horn