- Dillo-dev - lists.mailman3.com

BUG: CCC corrupts memory when the Content-Type is wrong
by onepoint＠starurchin.org Dec. 6, 2007

Dec. 6, 2007

I am hoping that someone who understands Dillo's CCC better than I can point me in the right direction and save me some time debugging this. Dillo almost always crashes if the HTML contains the stumbleupon.com favicon link, ie.'<img src="http://www.stumbleupon.com/favicon.ico">'. (This is very annoying as it makes Dillo unusable for one of my favourite blogs.) If it doesn't crash right away then clicking Reload crashes it. Examining core dumps with gdb suggests that memory is being corrupted, and running dillo under valgrind shows that the CCC functions are reading, writing and freeing already freed memory. (Details attached.) A local HTTP server triggers the bug when it server returns a Content-Type of "text/plain; charset=iso-8859-1". If I change it to "image/x-icon" there is no corruption. Accessing the image directly via a "file:" URL does not trigger the bug. (Strangely, using a local HTTP server does not hit the bug until I open a second window on the page, although sometimes it crashes first time.) I'm attaching a summary of the invalid memory accesses that valgrind reported, and the entire valgrind log (which includes stack traces). Any suggestions for where I should start instrumenting the CCC to find out why it is prematurely freeing pointers? Jeremy Henty

2 4

[patch] fix crashes after panel size changes
by Johannes.Hofmann＠gmx.de Dec. 6, 2007

Dec. 6, 2007

Hello, now that was a weird bug! I was experiencing crashes when I had changed the panel size via the right mouse on the search button. The problem is that FltkWidgetView::prepareCreateFltkWidget() was calling fltk::Group::begin(). Therefore after visiting pages with forms, the newly created panel was getting a child of Main instead of TopGroup! Btw. did I mention that this fltk begin()/end() stuff is evil? One could expect that Group::replace() should disconnect the new widget properly from its old parent (if there is any). But that's not the case. I wrote a mail to fltk.general about this. So in the end Main still had a reference to the panel and once it was deleted -- bang. The patch below uses Group::add() directly in FltkWidgetView::addFltkWidget() instead of FltkWidgetView::prepareCreateFltkWidget(). Does anyone remember why FltkWidgetView::prepareCreateFltkWidget() was added in the first place? Cheers, Johannes diff --git a/dw/fltkplatform.cc b/dw/fltkplatform.cc --- a/dw/fltkplatform.cc +++ b/dw/fltkplatform.cc @@ -83,10 +83,6 @@ FltkColor::FltkColor (int color, core::s colors[SHADING_DARK] = shadeColor (color, SHADING_DARK) << 8; colors[SHADING_LIGHT] = shadeColor (color, SHADING_LIGHT) << 8; } -} - -void FltkView::prepareCreateFltkWidget () -{ } void FltkView::addFltkWidget (::fltk::Widget *widget, diff --git a/dw/fltkplatform.hh b/dw/fltkplatform.hh --- a/dw/fltkplatform.hh +++ b/dw/fltkplatform.hh @@ -40,7 +40,6 @@ public: public: virtual bool usesFltkWidgets () = 0; - virtual void prepareCreateFltkWidget (); virtual void addFltkWidget (::fltk::Widget *widget, core::Allocation *allocation); virtual void removeFltkWidget (::fltk::Widget *widget); diff --git a/dw/fltkui.cc b/dw/fltkui.cc --- a/dw/fltkui.cc +++ b/dw/fltkui.cc @@ -103,7 +103,6 @@ void FltkResource::attachView (FltkView ViewAndWidget *viewAndWidget = new ViewAndWidget(); viewAndWidget->view = view; - view->prepareCreateFltkWidget (); viewAndWidget->widget = createNewWidget (&allocation); viewAndWidget->view->addFltkWidget (viewAndWidget->widget, &allocation); viewsAndWidgets->append (viewAndWidget); diff --git a/dw/fltkviewbase.cc b/dw/fltkviewbase.cc --- a/dw/fltkviewbase.cc +++ b/dw/fltkviewbase.cc @@ -375,17 +375,12 @@ bool FltkWidgetView::usesFltkWidgets () return true; } - -void FltkWidgetView::prepareCreateFltkWidget () -{ - begin (); -} - void FltkWidgetView::addFltkWidget (::fltk::Widget *widget, core::Allocation *allocation) { canvasWidgets->append (new TypedPointer < ::fltk::Widget> (widget)); allocateFltkWidget (widget, allocation); + add (widget); } void FltkWidgetView::removeFltkWidget (::fltk::Widget *widget) diff --git a/dw/fltkviewbase.hh b/dw/fltkviewbase.hh --- a/dw/fltkviewbase.hh +++ b/dw/fltkviewbase.hh @@ -82,7 +82,6 @@ public: int x, int y, int width, int height); bool usesFltkWidgets (); - void prepareCreateFltkWidget (); void addFltkWidget (::fltk::Widget *widget, core::Allocation *allocation); void removeFltkWidget (::fltk::Widget *widget); void allocateFltkWidget (::fltk::Widget *widget,

2 1

[patch] initialize mustQueueResize in Textblock constructor
by Johannes.Hofmann＠gmx.de Dec. 5, 2007

Dec. 5, 2007

Hello, this patch initializes mustQueueResize in the Textblock constructor. Valgrind was complaining about a jump depending on uninitialized memory. Cheers, Johannes diff --git a/dw/textblock.cc b/dw/textblock.cc --- a/dw/textblock.cc +++ b/dw/textblock.cc @@ -39,6 +39,7 @@ Textblock::Textblock (bool limitTextWidt line1Offset = 0; line1OffsetEff = 0; ignoreLine1OffsetSometimes = false; + mustQueueResize = false; /* * The initial sizes of lines and words should not be

2 1

[patch] add missing style->ref ()
by Johannes.Hofmann＠gmx.de Dec. 5, 2007

Dec. 5, 2007

Hello, the tiny patch below adds a missing style->ref() which was causing a lot of crashes on my system. cheers, Johannes diff --git a/dw/fltkui.cc b/dw/fltkui.cc --- a/dw/fltkui.cc +++ b/dw/fltkui.cc @@ -159,6 +159,7 @@ void FltkResource::setStyle (core::style this->style->unref (); this->style = style; + style->ref (); for (Iterator <ViewAndWidget> it = viewsAndWidgets->iterator (); it.hasNext(); ) {

2 1

[patch] partial redraws - updated
by Johannes.Hofmann＠gmx.de Dec. 4, 2007

Dec. 4, 2007

Hello, this patch fixes the problem pointed out by Sebastian regarding partial redraws and scrolling interference. It adds a new Region class that is implemented similarily as in dillo1 as a set of rectangles not containing each other. The conversion from canvas to view coordinates is now done at draw-time. Regards, Johannes PS: The code did not get as complex as I initially thought it would :-)

2 5

Re: patch: a _minuscule_ amount of cleaning up the header parsing code
by jcid＠dillo.org Dec. 3, 2007

Dec. 3, 2007

On Thu, Nov 29, 2007 at 03:08:12PM +0000, place wrote: > No change in behavior is intended. > Yes, that code is getting hard to understand. After a review, I wondered why Cache_parse_header() is dealing with the message body, when it should only parse the header, and why is it getting the (buf, bufsize) pair when it has the content in entry->Header. It looks that a cleaner way would be something like: (not tested, just pseudo-code) if (!(entry->Flags & CA_GotHeader)) { while ((len = Cache_get_header(entry, buf + start, buf_size - start))) { /* Let's scan, allocate, and set things according to header info */ Cache_parse_header(entry); if (entry->Flags & CA_GotHeader) { start += len; } else // Let the flow continue downward so the remaining // of the buffer is processed } return; } and to get rid of the: dbuf = a_Decode_process(entry->TransferDecoder, dbuf); dbuf = a_Decode_process(entry->ContentDecoder, dbuf); lines inside Cache_parse_header(). Please give it a try. -- Cheers Jorge.-

1 0

Re: charset conversion issues
by jcid＠dillo.org Nov. 28, 2007

Nov. 28, 2007

Hi, On Sat, Nov 24, 2007 at 10:50:18PM +0000, place wrote: > With rough code to look at. > No sense polishing up and completing what I have yet when you'll want changes... OK. > > > Charset in the meta tag. > My options seemed to be: > - Client->Callback returns information to cache. > Probably not supposed to happen. Too big of a change. Agreed. At least to postpone until other approaches prove messier. ;-) > - Reach the meta tag and undo/redo within html.cc. > Somewhat fragile. > Whatever I might come up with, some html out there would surely find a way > to outsmart me. This is the one I like most at first sight. Considering the charset can be given by HTTP or a META element: We can assume ASCII in the html text until the HEAD element is closed. If there's a charset in the META, then the decoder can be switched from null to the specified one. This approach has the advantage of working both when the charset comes via HTTP or META (<HEAD> content is ASCII). We can even add a text buffer for the HEAD element and append it to the whole HTML content if the offset is hard to set for the new decoder. Do the problems you found apply to this scheme too? > - Bail out and start again. > This is what I have now. In fact, not knowing at all what to do, I just > call a_Nav_cancel_expect() and a_Nav_push() right then and there. > Surprisingly, it functions instead of crashing, but I'm probably spilling > resources And Violating Principles. > (And, since a_Nav_push() isn't quite right for the situation even then, > I have to fix the nav stack ptr in Nav_cleanup()) > > > There's a dialog box to set the Content-Type for a page. If you change it > to text/plain, you get source in a proper window (with searching) instead > of the little fltk window. No line numbers, though. The idea of using Dillo for view source is interesting, i'm not sure though. Maybe a fltk widget (like an stripped-down editor as the one in FLTK's test/ directory) which gets utf-8 via a_Capi_get_utf8_data(Url) is also good. > > > Not wanting to mess with TypeDet and TypeHdr until finding out what you > think, I stuck in another one, TypeSet. I'd prefer to try to apply the second method (described above). > I'm directly calling a_Cache_[sg]et_content_type() from just wherever, > which may or may not be all right. Cache should be called from Capi only, but at this stage... > I knew that latin1 != UTF-8, but I was not thinking when I wrote > that Decode code. Must've been thinking about the Unicode code numbers. > FLTK has a workaround in it to deal with fools like me who send it latin1 > or cp1252, but better that I stop being foolish :) > > > I don't have Plain or View Source set up to decode yet. I don't know whether > the cache should know about decoding charsets. Or a Text base class or > something. > > > The real-type-matches-header-type code will have to learn to relax a bit, > now that html might contain all kinds of bytes... -- Cheers Jorge.-

2 1

patch: decode filter (Re: [Dillo-dev] Implemented a weighted apportionment algorithm for table rowspan.)
by jcid＠dillo.org Nov. 27, 2007

Nov. 27, 2007

On Mon, Nov 26, 2007 at 07:24:34PM +0000, place wrote: > > Yes. Send me this. > > All right. Attached. Committed. > > > > > 2) Chunked Transfer-Encoding running on top of (1) because I thought it might > > > help with gzip latency. It was a can-I-get-this-working-in-principle effort > > > and I will have to rewrite it now. Plus I need to actually read through the > > > http/1.1 spec (what I did was just based on a little web page that > > > summarized the main differences between 1.0 and 1.1 for me). > > > > Hmm, yes. Please rewrite it as simple/clean as possible, and with the SPEC > > in one hand. Once there, please send it. > > > > My experience is that code written to comply with the SPEC is easier > > to debug and polish (there're plenty of cases where wrong behaviour is on > > the server side, and thus we can be sure is not dillo). > > It sounds like somebody's indeed had his share of bitter experience with > being sent hacked-together patches :) I wasn't actually threatening to send > this version; just letting you know what's going on... Yes, you're right. 8-P > > The most annoying part so far was that 1.1 is supposedly able to send > "100 Continue" before you get the real stuff. I haven't seen one actually > do it yet, though. Maybe that's only if you're doing pipelining... > > Oh, and I just read about multiline headers. It wasn't really clear whether > this was new in 1.1, though. Did 1.0 have to handle it as well? Multiline headers?! sorry, I don't remember that off the top of my head. -- Cheers Jorge.-

2 2

Implemented a weighted apportionment algorithm for table rowspan.
by jcid＠dillo.org Nov. 26, 2007

Nov. 26, 2007

On Mon, Nov 26, 2007 at 03:34:09PM +0000, place wrote: > > Everybody: please enjoy this patch, it was a lot of work! :-) > > if you find a URL where it helps, I'd appreciate some feedback. > > Hurray for better tables! Pages on the wunderground.com site fit together better > than they used to. Thanks for the feedback. (though I didn't find a "Short cell" message in the first clicks). > I currently have: > 1) I finally had the excuse to make decode into a proper filter. That is, > Dstr* as input as well as output. Yes. Send me this. > 2) Chunked Transfer-Encoding running on top of (1) because I thought it might > help with gzip latency. It was a can-I-get-this-working-in-principle effort > and I will have to rewrite it now. Plus I need to actually read through the > http/1.1 spec (what I did was just based on a little web page that > summarized the main differences between 1.0 and 1.1 for me). Hmm, yes. Please rewrite it as simple/clean as possible, and with the SPEC in one hand. Once there, please send it. My experience is that code written to comply with the SPEC is easier to debug and polish (there're plenty of cases where wrong behaviour is on the server side, and thus we can be sure is not dillo). -- Cheers Jorge.-

1 0

Implemented a weighted apportionment algorithm for table rowspan.
by jcid＠dillo.org Nov. 26, 2007

Nov. 26, 2007

Hi there, A new, weighted apportionment algorithm for table rowspans was committed. This wasn't implemented in dillo2, and the algorithm is different from the one in dillo1. This produces much better table rendering when rowspan is used (if there's no rowspan in the table, it's the same as before). You can notice the rowspan case in the Web because of a stdout message like this one: "Short cell 0, sumRows=66 ih=110" It is interesting to compare dillo2.old, dillo2.rowspan, dillo1 and firefox. You can use: http://www.dillo.org/test/rowspan.html Sebastian: I added some comments in rounding-errors.doc, explained the algorithm, including a formula. I also corrected an index in the previous section. Please check it in rounding-errors.html after re-running doxygen, or in the attached patch (already in CVS). Notes: * In the test page above, the last table in the second set shows "cellspacing" is not working. AFAIU this is not implemented and subject to another patch. * The vertical alignment of a cell content is not yet implemented in dillo2. That's why the cell content is aligned at the top instead of the middle (default, as firefox does). This is subject of another patch. * The same algorithm may be used for width apportionment. I'm not sure on this though. Sebastian: are you planning to work on this soon? If not, I can try to work on the above mentioned problems. Advice welcomed! Everybody: please enjoy this patch, it was a lot of work! :-) if you find a URL where it helps, I'd appreciate some feedback. -- Cheers Jorge.-

2 2