Prevent NPE in HtmlAttributes.getIndex - #91
sideshowbarker wants to merge 1 commit into
Conversation
Apparently it’s possible for a “mode” value to be such that AttributeName.getQName(mode) returns null. So this change adds a null check to catch that. Otherwise, without this change, some documents cause that code to throw a NullPointerException.
|
That's really weird. I guess I will need to see in a debugger what's going on. |
The “vnu” branch of the htmlparser repo contains a workaround for this: validator/htmlparser#91 …and until we get the actual root cause of the problem figured out, that workaround is necessary in order to prevent the code from throwing a massive number of exceptions in production every day all day long.
|
The HtmlAttributes.getIndex NPE is getting thrown in the production by the W3C service every 5–10 minutes. And I get a notification every time. So I’ve created a https://github.com/validator/htmlparser/tree/vnu branch for the htmlparser repo and pushed the workaround from here to that branch — and in validator/validator@7d62ce1 I’ve switched the checker code over for now to using that htmlparser branch. |
|
OK, looking back through my notifications and logs for the documents that are causing the NPE to be thrown — and then manually running those documents back through the checker myself, I find that one thing they all so far appear to have in common is that in every case they’re also causing a “Stray start tag So, I can imagine that what may be happening is that the parser is trying to check the attributes on that stray |
|
OK, the document Some bisecting seems to indicate the change in 32c2256 introduced the regression for that case. If I build instead from 43aa7fc — the parent of that change — the NPE doesn’t get thrown. Looking briefly through the 32c2256 change, nothing obvious jumps out to me as being something that’d lead to the behavior breaking for the cc @emilio |
|
@sideshowbarker can you paste the full stack trace? |
Since it depends on it. Fixes validator#91
|
@sideshowbarker I think the right fix is #92, but if you could confirm it'd be appreciated. |
Yup, here it is in the context of the validator service: |
|
Superseded by #92 |
Since it depends on it. Fixes #91
3 participants
Apparently it’s possible for a
modevalue to be such thatAttributeName.getQName(mode)returns null. So this change adds a null check to catch that.Otherwise, without this change, some documents cause that code to throw a NullPointerException.
The NullPointerException is reproducible by parsing the document at https://usbliss.com/?attachment_id=2232 — or by giving that document to the https://validator.w3.org/nu/ checker to check:
https://validator.w3.org/nu/?doc=https://usbliss.com/?attachment_id=2232
That causes the checker to emit the following message:
(The admin who gets notified is me, and notification I get is a stack trace pointing to the NPE inHtmlAttributes.getIndex.)