Re: [patch] HTML parser bugfix
Jeremy Henty <onepoint-YprzHiG/[email protected]>
| Newsgroups | gmane.comp.web.dillo.devel |
|---|---|
| Message-ID | <[email protected]> |
123 wrote:
> On Sun, Jun 03, 2012 at 09:00:55PM +0100, Jeremy Henty wrote:
> > If you don't detect and ignore that extra double quote you will
> > break many pages that every other browser renders perfectly well.
>
> Then there should be some logic for detecting double quotes.
I agree. The hard question is: what logic?
> Searching for < inside quotes is not the right way as it breaks
> valid pages like reddit main page.
Again, I agree. In fact, I decided to experiment with removing
Dillo's misquote-detection just because it broke reddit.
Unfortunately it is very hard to come up with an algorithm that
correctly handles the various quoting horrors that you see all the
time, yet also correctly parses embedded javascript fragments like:
onclick='alert("clicked")'
(IIRC Dillo mis-renders reddit because of embedded javascript like
this.)
> Can you give examples of real web pages with double quotes?
I have attached a bunch of links that I collected after spending a lot
of time debugging pages that broke my locally-patched Dillo. (Ignore
the file:///... URLs as they obviously won't work for you.) I have
also attached the two patches I use. I think the first does the same
thing that you propose, although I haven't checked that in detail.
The second attempts to copy Firefox's misquote-detection algorithm.
I agree that Dillo's misquote-detection isn't good enough, but just
taking it out is a step backwards, and it is a mistake to propose
changes to it based solely on particular pages that Dillo mis-renders,
because any such change needs to be tested against the many other
broken pages that Dillo currently renders more or less correctly.
Regards,
Jeremy Henty
_______________________________________________
Dillo-dev mailing list
[email protected]
http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev
dillo.html
(text/html, 2.5 KB)
<li>Quote handling <ul> <li> <a href="http://the-witness.net/news/2011/12/engine-tech-concurrent-world-editing/"> Concurrent world editing</a> - text disappears because of an extra <code>"</code> inside an <code><a …></code> <li> <a href="http://www.cnas.org/blogs/abumuqawama/2011/04/quote-day.html-0"> Quote of the Day</a> - text disappears because of an attribute value that begins with two double quotes <li> <a href="http://cloggie.org/wissewords2/2011/01/25/young-cons-indeed/"> Young "cons" indeed</a> - text disappears because of an attribute value surrounded by a single quote and a double quote <li> <a href="http://www.itwire.com/opinion-and-analysis/open-sauce/44616-samba-4-will-break-desktop-monopoly"> iTWire - Samba 4 'will break desktop monopoly'</a> - text disappears because of an attribute value that has a trailing quote but no leading quote <li> <a href="http://scienceblogs.com/oscillator/2011/01/truth_stranger_than_fiction.php"> Truth Stranger Than Fiction</a> - most of the body disappears because of a <code><a href="http://liveness.org/plasticfutures/></code> <li> <a href="http://talklikeaduck.denhaven2.com/2011/01/16/they-say-its-your-birthday"> They Say It's Your Birthday?!</a> - most of the body disappears because of a <code><div class="meta""></code> <li> <a href="http://nielsenhayden.com/makinglight/archives/012795.html"> The Future is Here!</a> - the first 30 or so comments disappear because of a <code><TABLE border=0"></code> <li> <a href="http://blog.internetnews.com/skerner/2010/12/openbsd-backdoored-by-the-fbi.html"> OpenBSD backdoored by the FBI?</a> - a <code>'2px"</code> in a tag causes most of the page to be skipped. <li> <a href="file:///home/jeremy/Personal/Geeky/Dillo/Test/quote_missing_initial.html"> Missing initial quote on an attribute</a> - causes the body to disappear. <ul> <li> <a href="http://www.dwheeler.com/program-library/"> Online example</a> <li> <a href="http://www.itwire.com/opinion-and-analysis/open-sauce/45391-ubuntu-there-was-never-any-love-to-start-with"> Online example</a> <li> <a href="http://www.itwire.com/opinion-and-analysis/open-sauce/45585-kororaa-gnulinux-is-back"> Online example</a> </ul> <li> <a href="file:///home/jeremy/Personal/Geeky/Dillo/Test/quote_backslash_escaped.html"> Embedded backslash-escaped quote</a> - causes some of the content of a <code><script></code> element to appear in the body. <ul> <li> <a href="http://libregraphicsworld.org/articles.php?article_id=23"> Online example</a> </ul> </ul>
html_parse_quoted_attributes_as_per_strict_html5
(text/plain, 1.4 KB)
# HG changeset patch # User Jeremy Henty <onepoint-YprzHiG/[email protected]> # Date 1294509336 0 # Node ID 39425e6e4ea9a881434ae191a60292c395f0c764 # Parent aa659f60265bd35f65d22f30fde65bb39e3ec2ac HTML: parse quoted attributes as per strict HTML5. diff --git a/src/html.cc b/src/html.cc --- a/src/html.cc +++ b/src/html.cc @@ -3786,20 +3786,7 @@ /* Skip over quoted string */ buf_index++; buf_index += strcspn(buf + buf_index, - (ch == '"') ? "\">" : "'>"); - if (buf[buf_index] == '>') { - /* Unterminated string value? Let's look ahead and test: - * (<: unterminated, closing-quote: terminated) */ - int offset = buf_index + 1; - offset += strcspn(buf + offset, - (ch == '"') ? "\"<" : "'<"); - if (buf[offset] == ch || !buf[offset]) { - buf_index = offset; - } else { - BUG_MSG("attribute lacks closing quote\n"); - break; - } - } + (ch == '"') ? "\"" : "'"); } else if (ch == '<') { /* unterminated tag detected */ p = dStrndup(buf+token_start+1,
html_parse_quoted_attributes_as_per_firefox
(text/plain, 3 KB)
# HG changeset patch # User Jeremy Henty <onepoint-YprzHiG/[email protected]> # Date 1294509336 0 # Node ID c030a92ee6c609854d745bc9025c1af4f86c36c8 # Parent 9d72bcf5c4593b5ed028896e866f2b1b90bc23be HTML: parse quoted attributes as per Firefox. diff --git a/src/html.cc b/src/html.cc --- a/src/html.cc +++ b/src/html.cc @@ -3570,7 +3570,8 @@ const char *attrname, int tag_parsing_flags) { - int i, isocode, entsize, Found = 0, delimiter = 0, attr_pos = 0; + int i, isocode, entsize, Found = 0, attr_pos = 0, + delimiter = 0, delimiter_index; Dstr *Buf = html->attr_data; DilloHtmlTagParsingState state = SEEK_ATTR_START; @@ -3607,8 +3608,13 @@ break; case SEEK_VALUE_START: if (!isspace(tag[i])) { - delimiter = (tag[i] == '"' || tag[i] == '\'') ? tag[i] : ' '; - i -= (delimiter == ' '); + if (tag[i] == '"' || tag[i] == '\'') { + delimiter = tag[i]; + delimiter_index = i; + } else { + delimiter = ' '; + --i; + } state = (Found) ? GET_VALUE : SKIP_VALUE; } break; @@ -3616,11 +3622,22 @@ case SKIP_VALUE: if ((delimiter == ' ' && isspace(tag[i])) || tag[i] == delimiter) state = SEEK_TOKEN_START; + else if (i == tagsize - 1 + && (delimiter == '"' || delimiter == '\'')) { + /* no closing delimiter */ + i = delimiter_index; /* rewind to the opening delimiter */ + delimiter = ' '; /* parse an unquoted string */ + } break; case GET_VALUE: if ((delimiter == ' ' && (isspace(tag[i]) || tag[i] == '>')) || tag[i] == delimiter) { state = FINISHED; + } else if (i == tagsize - 1 + && (delimiter == '"' || delimiter == '\'')) { + /* no closing delimiter */ + i = delimiter_index; /* rewind to the opening delimiter */ + delimiter = ' '; /* parse an unquoted string */ } else if (tag[i] == '&' && (tag_parsing_flags & HTML_ParseEntities)) { if ((isocode = Html_parse_entity(html, tag+i, @@ -3786,9 +3803,13 @@ break; } else if (ch == '"' || ch == '\'') { /* Skip over quoted string */ - buf_index++; + int quoted_index = ++buf_index; buf_index += strcspn(buf + buf_index, (ch == '"') ? "\"" : "'"); + if (Eof && buf_index == bufsize) + /* no closing delimiter, + * rewind to the start of the quoted string */ + buf_index = quoted_index; } else if (ch == '<') { /* unterminated tag detected */ p = dStrndup(buf+token_start+1,