search

google / gumbo-parser

An HTML5 parsing library in pure C99
Apache License 2.0
5.16k stars 661 forks source link

invalid attribute names should be ignored #415

Closed vinc17fr closed 1 year ago

vinc17fr commented 4 years ago

Invalid attribute names should be ignored. This includes unexpected or duplicate quote mark. For instance, consider the following cases: <body "> and <body !=foo>. With the callback (used via the HTML::Gumbo Perl module), events associated with these invalid attribute names are generated, while no attributes should be generated in such a case. Compare with what HTML Tidy does.

In practice, I got the error when I wanted to parse the Inria home page, which currently contains: <body class="path-frontpage has-glyphicons noTouchDevice"">

craigbarnes commented 4 years ago

Compare with what HTML Tidy does.

Compare with what Chrome, Firefox, etc. do.

A general purpose parsing library should follow the specification, not aim to replicate some other tool's behaviour.

vinc17fr commented 4 years ago

Chrome, Firefox, etc. are third-party browsers. HTML Tidy comes from the W3C and is meant to follow the specifications. That's why I've mentioned it.

craigbarnes commented 4 years ago

The relevant parts of the spec are:

From what I can see, Gumbo is doing exactly what the specification requires in this case.

The HTML Tidy behaviour may actually be useful, since it's purpose is to "tidy" bad HTML, but it's not "following the specifications" here like a user agent is supposed to.

craigbarnes commented 4 years ago

By the way, I'm just commenting as an outside party. This project has been more or less abandoned for ~4 years.

vinc17fr commented 4 years ago

In any case, gumbo generates an invalid attribute name: 12.1.2.3 Attributes. This is not correct.

craigbarnes commented 4 years ago

In any case, gumbo generates an invalid attribute name: 12.1.2.3 Attributes.

See the note above for §12.1:

§12.1 Writing HTML documents

This section only applies to documents, authoring tools, and markup generators. In particular, it does not apply to conformance checkers; conformance checkers must use the requirements given in the next section ("parsing HTML documents").

Requirements for authors and requirements for parsers are not the same thing. The spec encourages authors to write "well formed" documents. However, parsers have to be much more lenient in what they accept, due to a massive legacy of existing web content.

This is not correct.

Following the steps given for parsers, it seems quite correct to me.

There are GumboError and GumboParseFlags types that are recorded during parsing. They should encode the various errors and unusual conditions mentioned in the spec. There's sufficient information encoded in the parse tree to determine which nodes were ill-formed, parser-inserted, etc. but it takes some extra effort to use it.

vinc17fr commented 4 years ago

Invalid attribute names make the XML::LibXML Perl module crash. There needs to be some method to convert HTML5 to XHTML5 without requiring the converter to perform additional well-formedness checking. I'm using the HTML::Gumbo Perl module (with the callback method), and information about the errors is not available there. Perhaps the issue is in this module, then.

kevinhendricks commented 4 years ago

if what you want is xhtml from html5 and want info on errors, there are other forks of gumbo that are actively maintained, that will accept xhtml and html inputs, serialize a parse tree back to xhtml, and provide parse error info.

One is used in the epub editor and one is used in Calibre. See here for the Sigil fork (I maintain it and will respond to error reports)

https://github.com/Sigil-Ebook/sigil-gumbo

and Calibre's is here: ( Kovidgoyal maintains it and does respond to bug reports as well).

https://github.com/kovidgoyal/html5-parser

and both of these have borrowed heavily from the many many bug fixes and developments/speedups done by many of the people from this site.

If it helps, both of the above have python3 bindings and work well with bs4 and are actively used in epub related projects.

kevinhendricks commented 4 years ago

FWIW, trying a callback approach on parsing non-xml, non-strict html with an html5 spec compliant parser is going to be a pain as the spec calls for specific errors, injected nodes, reordering and reparenting and even resequencing poorly nested html, etc.

You may want to post process the already built tree. This is how the bindings to python do it. See Calibre's implementation that binds to python for example.

vinc17fr commented 4 years ago

I've opened a Debian bug to ask for a replacement by sigil-gumbo.

With the HTML::Gumbo Perl module, the tree format is in alpha stage. That's why I did not choose this method.

kevinhendricks commented 4 years ago

You may want to modify that to choose the html5-parser as it will show a better way to bind to a language like Perl (as it is doing the same for Python).

Since Sigil is a C++ project, we use the gumbo built tree directly and added our own post build simple tree modification ability and then wrappered it in a C++ class to make all of the higher level functionality typically found in a DOM library.

craigbarnes commented 4 years ago

You may want to modify that to choose the html5-parser

I was under the impression that html5-parser only offers a Python API? Do they support building and installing libgumbo for library use too?

It seems to me like none of the half dozen or so forks actually support being linked as a shared library. I don't think there's anything that can really replace a libgumbo distro package, unless I'm mistaken about html5-parser.

kevinhendricks commented 4 years ago

I thought he was trying to build a perl binding to gumbo. Since building gumbo as a shared library is quite easy to do with cmake, I thought seeing how they create the interface binding to python would be more useful than seeing our sigil-gumbo library being used in C++ classes. Using a callback approach on top of gumbo is just not going to work easily to create nodes on the fly for processing non-strict html unless I am missing something.

Sorry if I assumed wrong....

Kevin

On Feb 8, 2020, at 8:53 PM, Craig Barnes notifications@github.com wrote:

 You may want to modify that to choose the html5-parser

I was under the impression that html5-parser only offers a Python API? Do they support building and installing libgumbo for library use too?

It seems to me like none of the half dozen or so forks actually support being linked as a shared library. I don't think there's anything that can really replace a libgumbo distro package, unless I'm mistaken about html5-parser.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

craigbarnes commented 4 years ago

I'm not sure about the details of this particular use case. I just recall seeing html5-parser described as "... HTML 5 parsing for python", so presumably they don't support linking to their internal copy of Gumbo as a shared library. Of course, that's not to say it couldn't be taken and adapted. It just doesn't seem like an out-of-the-box solution, if that's what's being sought.

Using a callback approach on top of gumbo is just not going to work easily to create nodes on the fly for processing non-strict html unless I am missing something.

Yeah, a callback approach with Gumbo makes no sense whatsoever. The Hubbub library offers a streaming/callback interface, but IMO the API is horrible compared to Gumbo (albeit for legitimate reasons).

kevinhendricks commented 4 years ago

Good point. What about your own fork?

There must be 3 or 4 active forks out there all doing something a bit different. It is a real shame they did not pass ownership of this project to someone to keep it alive or at least pull in the bug fixes that people have posted about here.

craigbarnes commented 4 years ago

What about your own fork?

I forked the library because it seemed like the best route to fixing the bugs that affected lua-gumbo. I'd probably contribute to a proper fork of the library, if one were to emerge, but I don't really want to lead such an effort.