Replace vendored html5lib with stdlib

[jdufresne]

The html5lib library isn't strictly required as the same functionality
can be achieved through the stdlib html.parser module.

The html5lib is one of the largest uses of the six library. By dropping
this unnecessary dependency, the pip project is closer to dropping the
six library.

Additionally, html5lib maintenance has slowed down and the project has
rejected pull requests to drop Python 2 support.

For now, the html5lib code remains, but is hidden behind the command
line options: --use-deprecated=html5lib. After a sufficient amount of
time has passed without any reported bugs, the vendored library can be
removed completely.

[uranusjr]

the same functionality can be achieved through the stdlib html.parser module

I have not read your implementation, but html5lib covers a ton of edge cases that html doesn’t that we’ll need to replicate. If we’re dropping html5lib, I’d prefer doing it with a new library that is able toe replicate html5lib (e.g. passing its test suite or some kind of HTML5 spec validator) and vendor it in. HTML is a very hairy format to parse.

[pfmoore]

While I do have some concerns here (mostly along the lines of "why did we vendor html5lib in the first place if that is all we use it for?") I will say that the replacement code (which I've only skimmed so far) is based on the stdlib html.parser module and I think it's entirely reasonable for us to assume that's a valid HTML parser (it claims to be a "simple HTML and XHTML parser", but I don't see why "simple" should be taken to mean "potentially buggy").

In particular, the index format is intended to be relatively straightforward to parse, so I think it's reasonable to expect the stdlib parser to work¹. And honestly, I'm not worried about possible security vulnerabilities - if we're not entitled to assume that the Python stdlib has no security vulnerabilities, we're pretty much stuffed.

In principle, I'm +1 on this change.

(My other motive here is that from reading a couple of the html5lib issues, it feels like removing our dependency would relieve a certain amount of pressure on the html5lib maintainer(s) who don't really want to be "that person in https://xkcd.com/2347/")

¹ I'd go so far as to be OK with amending PEP 503 to say that an index page must be parsable using the stdlib parser, if that helped...

[jdufresne]

I have not read your implementation, but html5lib covers a ton of edge cases that html doesn’t that we’ll need to replicate. If we’re dropping html5lib, I’d prefer doing it with a new library that is able toe replicate html5lib (e.g. passing its test suite or some kind of HTML5 spec validator) and vendor it in. HTML is a very hairy format to parse.

The code using html5lib doesn't care about standard compliant HTML5 so long as it is parsable as HTML. It is merely looking for <a> and <base> elements which the stdlib HTML parser is suited to doing. If those <a> elements happen to exist in an illegal context (as far as WHATWG is concerned) then I don't see why pip really cares, just read the HREF.

If there is specific HTML that we know will not be handled by stdlib but is handled by html5lib, then I think we should get it in test suite regardless of how this PR plays out. It would make proof-of-concepts easier in the future. Do you have one such example? I do not.

why did we vendor html5lib in the first place if that is all we use it for?

html5lib was first introduced in PR #973. This PR states: "Use html5lib to parse HTML instead of using regexs". So, it looks to me like stdlib was either less capable in 2013 or it was not considered at that time.

[uranusjr]

Maybe we should ask @dstufft (IIRC he explained somewhere why html5lib is better a while ago)

The code using html5lib doesn't care about standard compliant HTML5 so long as it is parsable as HTML.

IIRC html5lib already does that (the reason it’s called html5lib in the first place), and if we remove it we’ll need to do some validation ourselves.

[jdufresne]

The code using html5lib doesn't care about standard compliant HTML5 so long as it is parsable as HTML.

IIRC html5lib already does that (the reason it’s called html5lib in the first place), and if we remove it we’ll need to do some validation ourselves.

Sorry, I'm not sure I follow the point. In response to "The code using html5lib doesn't care about standard compliant HTML5 so long as it is parsable as HTML" you refer to validation, but I don't know what specific validation is a concern to pip. pip just needs to find a elements. I appreciate that html5lib is more sophisticated, I'm just not sure that matters to pip. Where does that behavior come into play for merely finding a elements?

Can you provide a specific example of invalid HTML document or fragment where pip with html5lib does the right but stdlib fails? I'd like to get such an example in the test suite.

[sbidoul]

Perhaps we could do a release that switches to the stdlib parser by default and has --use-deprecated=html5lib (or some better flag name that does not make people think that html5lib itself is deprecated).

[uranusjr]

Sorry, I'm not sure I follow the point. In response to "The code using html5lib doesn't care about standard compliant HTML5 so long as it is parsable as HTML" you refer to validation, but I don't know what specific validation is a concern to pip. pip just needs to find a elements. I appreciate that html5lib is more sophisticated, I'm just not sure that matters to pip. Where does that behaviour come into play for merely finding a elements?

The Simple Repository API specification (PEP 503) says the page must be a valid HTML5 document. Pip’s parsing logic only finds <a> tags because html5lib already performs that validation for us, so when pip’s code is accessed, we already know we’re dealing with a valid HTML5 and can simply proceed onto other parts of the API. So if we drop html5lib, pip has two choices: The first is to stick to the specification and re-implement those HTML5 validations. This is not technically difficult, but also arguably should be a separate lib.

The other, easier route for pip is to forgo that validation part. This means pip will start to accept technically invalid PEP 503 inputs, which is similar to web browsers implements JavaScript not conforming to specs. Since pip is to Python packaging like Google Chrome is to web right now (or IE if you’re old like me), we deliberately accepting bad inputs would allow them be able to say “but pip accepts this” and put alternative PEP 503 clients in an awkward position. This is what I want to avoid.

[sbidoul]

Another aspect to consider is performance.

[pfmoore]

The other, easier route for pip is to forgo that validation part.

Honestly, my view is that it's not up to pip to do the validation, and we have every right to consider the behaviour if users supply a non-conformant index as being "undefined". It's a position we've been pushing relatively consistently (the idea that pip's behaviour is defined by the standards, not the other way around) so I think this fits that approach.

I guess if it really bothers you, we could add

def handle_decl(self, decl):
    # Fail more gracefully here if you want
    assert decl == "DOCTYPE html"

It's not exactly difficult - and I'm definitely OK with blaming the user if their index claims to be HTML 5 but isn't...

[jdufresne]

@uranusjr Thanks for the deeper explanation. I understand your POV better now.

FWIW, html5lib is a non-validating parser. Here is an unrelated issue I filed back in 2016 to demonstrate: html5lib/html5lib-python#283

With some effort, you can find other such examples that parse in html5lib and do not pass an HTML5 validator such as https://validator.w3.org/nu/.

[dstufft]

Doesn't html.parser fail on some well formed HTML5 output? I could be remembering wrong, but I thought that was a problem with it.

[pfmoore]

That would be an issue, but without specifics I'd like to assume that counts as a CPython bug (and quite possibly one that's been fixed by now). Also, while it's technically acceptable for an index to serve arbitrary HTML, realistically we're only likely to see relatively straightforward content.

I think we can probably assume it's fine and deal with bug reports if they happen (although I do agree it might be reasonable to provide a deprecation period so we can tease out such cases - although my instinct is we won't see any).

[pradyunsg]

My 2 cents: Let's do this and keep html5lib-based parsing behind a flag for a release.

Check that the decl is correct should be sufficient IMO. I don't expect we'd see any show-stoppers, and if someone is passing horrible pages to pip, well, IMO that's a problem we can deal with when it happens.

I definitely empathise with the argument that "what pip accepts == what works" is how stuff works, but there's absolutely no reason to believe that index pages will have sufficent edge-cases to behave incorrectly.

We should probably at least keep the html5lib codepath for 3 months (i.e. one release) behind a --use-deprecated=... to act as an escape hatch for someone affected by this. That way, we'd at least have a vent for letting the not-so-noisy users pass through without pain.

[jdufresne]

I can explore the deprecation route and incorporate the other suggestions. Moving to draft until I have some time to look into it.

[uranusjr]

I’m suddenly remembering there’s actually already an implementation for this: https://github.com/brettcannon/mousebender

Since iirc @brettcannon already expressed interest including this in packaging, I think he’s probably not that too worried being that person living in Nebraska 🙂

I think we can even drop some more logic in pip if we use mousebender.

[brettcannon]

There's pypa/packaging#424 to upstream the Simple Index code from mousebender to 'packaging', but it never got much traction. I'm still up for seeing that happen to get more support from folks and so I can experiment in other things in mousebender and to not have any dependency limitations.

But if you all want to depend on mousebender directly then let me know and I can check with my co-maintainer on how he feels about that. We would probably switch to CalVer and minimize external dependencies in that instance.

[jdufresne]

Changes:

• Rebased on latest main branch
• Added the doctype checker as suggested in Replace vendored html5lib with stdlib #10291 (comment)
• Added the --use-deprecated=html5lib command line option to allow a deprecation period
• Added some additional tests around the new changes
• Added a news entry

I believe all feedback has been addressed, so this is ready for more review. Thanks!

[jdufresne]

Thanks @uranusjr and @pradyunsg. I applied the latest suggestions.

[uranusjr]

We should also tell the user to report this somewhere. (Let's do it when we decide this is ready for merge.)

[pradyunsg]

FWIW, yet-another reason to do this is that the html5lib dependency is the blocker for us to drop chardet, which is the largest dependency that we vendor, and also helps progress toward #9824's motivating licensing concerns; now that requests supports using charset_normalizer. :)

[pradyunsg]

@jdufresne Could you rebase this on the current main and include a link to #10825, in the spot requested by @uranusjr?

[pradyunsg]

FYI: the tests aren't happy.

[jdufresne]

Sorry about that.

@pradyunsg Are you able to put the finishing touches on this PR? Sorry, but I no longer have the time to dedicate to this work. If that means it won't make the release, I understand.

[pradyunsg]

Neato. I'll try and pick this up in time for release.

[pradyunsg]

Check that the decl is correct should be sufficient IMO. I don't expect we'd see any show-stoppers, and if someone is passing horrible pages to pip, well, IMO that's a problem we can deal with when it happens.

I definitely empathise with the argument that "what pip accepts == what works" is how stuff works, but there's absolutely no reason to believe that index pages will have sufficent edge-cases to behave incorrectly.

Ahahahah, looks like nearly everyone except PyPI is passing non-compliant pages to pip. :(