classification
Title: HTMLParser: parsing error
Type: behavior Stage: committed/rejected
Components: Library (Lib) Versions: Python 3.3, Python 3.2, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ezio.melotti Nosy List: Jim.Jewett, Michel.Leunen, eric.araujo, ezio.melotti, georg.brandl, python-dev, r.david.murray, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2012-04-09 20:01 by Michel.Leunen, last changed 2012-04-19 11:55 by Michel.Leunen. This issue is now closed.

Files
File name Uploaded Description Edit
issue14538.diff ezio.melotti, 2012-04-12 07:48 Patch against 2.7
issue14538-2.diff ezio.melotti, 2012-04-12 23:26 Patch against 2.7
Messages (18)
msg157890 - (view) Author: Michel Leunen (Michel.Leunen) Date: 2012-04-09 20:01
HTMLParser fails to parse this structure of tags: 

'<a></a><script></script><meta><meta / ><body></body>'

Parsing stops after the first meta tag ignoring the remainers

from HTMLParser import HTMLParser
parser = process_html()
parser.feed('<a></a><script></script><meta><meta / ><body></body>')

Python 2.7.2+ Ubuntu 11.10
msg157899 - (view) Author: Jim Jewett (Jim.Jewett) Date: 2012-04-09 20:50
What do you think it should do?

My thought is that meta tags may or may not be void, but certainly should not be nested.  As XML, I would parse that as 

<a></a>
<script></script>
<meta>
    <meta / >
    <body></body>
*missing closing tag

But for html, there is more cleanup.  The catch is that this module probably can't keep up with BeautifulSoup or HTML5lib ... is this particular tag situation common enough to be even have a defined handling, let alone one that is worth adding to a "simple" module?
msg157906 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-04-09 21:39
With Python 2.7.3rc2 and 3.3.0a0 (strict=False) I get:

Start tag: a
End tag  : a
Start tag: script
End tag  : script
Start tag: meta
Data     : <meta / >
Start tag: body
End tag  : body

This is better, but still not 100% correct, the "<meta / >" shouldn't be seen as data.
msg158109 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-04-12 07:22
ISTM that "<meta / >" is neither valid HTML nor valid XHTML.
msg158111 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-04-12 07:48
Here's a patch.
msg158130 - (view) Author: Jim Jewett (Jim.Jewett) Date: 2012-04-12 14:03
-1 on that particular patch.  

<tagname / >


(with only whitespace between "/" and ">") strikes me as obviously intending to close the tag, and a reasonably common error.

I can't think of any reason to support nested meta tags while not supporting sloppy self-closing tags.
msg158132 - (view) Author: Jim Jewett (Jim.Jewett) Date: 2012-04-12 14:09
This issue is also marked for (bugfix-only) 2.7 and 3.2.  

Unless there is a specification somewhere (or at least an editor's draft), I can't really see any particular parse as a bugfix.  Was the goal just to make the parse finish, as opposed to stopping part way through the text?
msg158140 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-04-12 14:58
To be consistent, this patch should remove the references to http://www.w3.org/TR/html5/tokenization.html#tag-open-state and http://www.w3.org/TR/html5/tokenization.html#tag-open-state as irrelevant.
msg158143 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-04-12 15:26
Yes, after considerable discussion those of working on this stuff decided that the goal should be that the parser be able to complete parsing, without error, anything the typical browsers can parse (which means, pretty much anything, though that says nothing about whether the result of the parse is useful in any way).  In other words, we've been treating it as a bug when the parser throws an error, since one generally uses the library to parse web pages from the internet and having the parse fail leaves you SOL for doing anything useful with the bad pages one gets therefrom.  (Note that if the parser was doing strict adherence to the older RFCs our decision would have been different...but it is not.  It has always accepted *some* badly formed documents, and rejected others.)

Also note that BeautifulSoup in Python2 used the sgml parser, which didn't throw errors, but that is gone in Python3.  In Python3 BeautifulSoup uses the html parser...which is what started us down this road to begin with.
msg158168 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-04-12 21:03
I apologize for misplaced sarcasm. After more careful reading of the source code, I found out that the patch really meets the specifications and the behavior of all tested me browsers. Despite its awkward appearance, the patch fixes a flaw of the original code for this corner case. But it allows the passage of an invalid code in strict mode. I think, this problem can be solved by a more direct way, perhaps even simplifying the original code.
msg158185 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-04-12 23:26
Attached a new patch with a few more tests and a simplification of the attrfind regex.


> But it allows the passage of an invalid code in strict mode.

HTMLParser is following the HTML5 specs, and doesn't do validation, so there's no strict mode (and the strict mode of Python 3 will be removed/deprecated soon).

> I think, this problem can be solved by a more direct way,
> perhaps even simplifying the original code.

This might be true, but I'm quite happy with this patch for now.  Maybe one day I'll rewrite it.


> Unless there is a specification somewhere (or at least an editor's
> draft), I can't really see any particular parse as a bugfix.

If HTMLParser doesn't parse as the HTML5 specs say, then it's considered a bug.
msg158195 - (view) Author: Jim Jewett (Jim.Jewett) Date: 2012-04-13 02:55
On Thu, Apr 12, 2012 at 7:26 PM, Ezio Melotti wrote:
> If HTMLParser doesn't parse as the HTML5 specs say,
> then it's considered a bug.

I had thought that was the goal of the html5lib, and that HTMLParser
was explicitly aiming at a much reduced model in order to remain
simple.  (And also because the HTML5 spec is still changing fairly
often, particularly compared to the lifecycle of a feature release of
python.)
msg158198 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-04-13 06:20
HTMLParser is still simpler than html5lib, but if/when possible we are following the HTML5 standard rather than taking arbitrary decisions (like we used to do before HTML5).  HTMLParser doesn't claim to be a fully compliant HTML5 parser (and probably never will) -- its goal is rather being able to parse real world HTML in the same way browsers do.  There are also a couple of corner cases that are not implemented in HTMLParser because it would make the code too complicated for a little gain.
msg158215 - (view) Author: Jim Jewett (Jim.Jewett) Date: 2012-04-13 17:34
It sounds like this is a case where the docs should mention an external library; perhaps something like changing the intro of http://docs.python.org/dev/library/html.parser.html from:

"""
19.2. html.parser — Simple HTML and XHTML parser
Source code: Lib/html/parser.py

This module defines a class HTMLParser which serves as the basis for parsing text files formatted in HTML (HyperText Mark-up Language) and XHTML.
"""

to:


"""
19.2. html.parser — Simple HTML and XHTML parser
Source code: Lib/html/parser.py

This module defines a class HTMLParser which serves as the basis for parsing text files formatted in HTML (HyperText Mark-up Language) and XHTML.  

Note that mainstream web browsers also attempt to repair invalid markup; the algorithms for this can be quite complex, and are evolving too quickly for the Python release cycle.  Applications handling arbitrary web pages should consider using 3rd-party modules.  The python version of html5lib ( http://code.google.com/p/html5lib/ ) is being developed in parallel with the HTML standard itself, and serves as a reference implementation.
"""
msg158220 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-04-13 17:50
Sure, the docs should explain better that html.parser tries its best to parse stuff, is not a validating parser, and is actively developed, contrary to the popular belief that standard library modules never get improved.  I’m less sure about links, there is more than one popular library (html5lib, lxml, BeautifulSoup).  Another bug needs to be opened, we’re off-topic for this one — but thanks for the remark and proposed patch.
msg158695 - (view) Author: Roundup Robot (python-dev) Date: 2012-04-19 01:36
New changeset 36c901fcfcda by Ezio Melotti in branch '2.7':
#14538: HTMLParser can now parse correctly start tags that contain a bare /.
http://hg.python.org/cpython/rev/36c901fcfcda

New changeset ba4baaddac8d by Ezio Melotti in branch '3.2':
#14538: HTMLParser can now parse correctly start tags that contain a bare /.
http://hg.python.org/cpython/rev/ba4baaddac8d

New changeset 0f837071fd97 by Ezio Melotti in branch 'default':
#14538: merge with 3.2.
http://hg.python.org/cpython/rev/0f837071fd97
msg158696 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-04-19 01:45
This is now fixed, thanks for the report!

Regarding the documentation feel free to open another issue, but at some point I'll probably update it anyway and/or write down somewhere what the future plans for HTMLParser and its goals.
msg158718 - (view) Author: Michel Leunen (Michel.Leunen) Date: 2012-04-19 11:55
Thanks guys for your comments and for solving this issue. Great work!
History
Date User Action Args
2012-04-19 11:55:29Michel.Leunensetmessages: + msg158718
2012-04-19 01:45:30ezio.melottisetstatus: open -> closed
resolution: fixed
messages: + msg158696

stage: patch review -> committed/rejected
2012-04-19 01:36:30python-devsetnosy: + python-dev
messages: + msg158695
2012-04-13 17:50:31eric.araujosetnosy: + eric.araujo
messages: + msg158220
2012-04-13 17:34:02Jim.Jewettsetmessages: + msg158215
2012-04-13 06:20:33ezio.melottisetmessages: + msg158198
2012-04-13 02:55:34Jim.Jewettsetmessages: + msg158195
2012-04-12 23:26:37ezio.melottisetfiles: + issue14538-2.diff

messages: + msg158185
2012-04-12 21:03:52serhiy.storchakasetmessages: + msg158168
2012-04-12 15:26:45r.david.murraysetnosy: + r.david.murray
messages: + msg158143
2012-04-12 14:58:19serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg158140
2012-04-12 14:09:12Jim.Jewettsetmessages: + msg158132
2012-04-12 14:03:56Jim.Jewettsetmessages: + msg158130
2012-04-12 07:48:31ezio.melottisetfiles: + issue14538.diff
keywords: + patch
messages: + msg158111

stage: test needed -> patch review
2012-04-12 07:22:31georg.brandlsetnosy: + georg.brandl
messages: + msg158109
2012-04-09 21:39:38ezio.melottisetassignee: ezio.melotti
stage: test needed
messages: + msg157906
versions: + Python 3.2, Python 3.3
2012-04-09 21:19:23pitrousetnosy: + ezio.melotti
2012-04-09 20:50:55Jim.Jewettsetnosy: + Jim.Jewett
messages: + msg157899
2012-04-09 20:04:38Michel.Leunensettitle: HTMLParser -> HTMLParser: parsing error
2012-04-09 20:01:55Michel.Leunencreate