Potential bug in html comment output

[geoffmcl]

Still exploring this, but was uncovered in issue #643...

If the config is char-encoding: latin1, and an invalid character, say 0x96, decimal 150, is found in a html comment, when tidy encounters that 0x96, it uses DecodeWin1252 to get the unicode 0x2013, 8211, to store in the lexer, and outputs a warning, INVALID_SGML_CHARS, line ?? column ?? - Warning: replacing invalid character code 150...

But when that comment text, with that substitute character 0x2013, 8211, is output to the html file, it will be raw encoded as 0x13 in the document...

Or maybe as 0x13 0x20, or maybe 0x20 0x13, but the point is the 0x13, decimal 19, makes it into the document output stream... This is a non-printable DC3 character, and has no place in any text document...

A sample html document, with this 0x96 character, can be found in our regression test repo, case-445557.html. The first - after the word had in the 2nd line comment...

Of course make sure you use char-encoding: latin1 in testing, although it may exist even when other character encodings are used...

As stated, still to fully explore and understand this... any help appreciated... thanks...

[balthisar]

@geoffmcl, this is send alarm bells ringing, because I remember encountering an issue similar to this, maybe last year, and for the life of me I can't find my notes on the matter.

Okay, after meticulously combing the commit history, I found it: #339. And do you want to guess about which test case it addresses? Yeah, 445557.html.

My memory is shaky here, but I seem to recall that that old, ancient, fix produced identical output on macOS as Windows and Linux (which was the goal), but that when looking at this case in the Windows console, there was an extra linefeed that Unixes ignored. Given that I didn't want to fiddle with anything else, I never followed up on why this extra character was present. I do remember seeing the extra byte in a binary editor, though.

[geoffmcl]

@balthisar thank you for the feedback...

I have now read #339, and surprised it seems I did not comment at the time, but it certainly seems to sort of touch the same issue... maybe I was asleep, or on holiday, or something...

I have now pushed a simple sample in_651.html, and a config cfg_651.txt to my repo for testing... This sample contains 2 0x96 characters, one in a comment, and the other in a paragraph...

First aside from the warning that the document uses legacy encoding, which can be filtered, it passes the validator... that raises some interesting questions...

However, running current tidy, raises some more interesting questions... the output -

F:\Projects\tidy-test\test>tidy5 -config input5\cfg_651.txt -o temp-651.html input5\in_651.html
line 8 column 16 - Warning: replacing invalid character code 150
line 9 column 14 - Warning: replacing invalid character code 150
line 4 column 1 - Info: <meta> attribute "charset", incorrect value "windows-1252" replaced
Tidy found 2 warnings and 0 errors!

Character codes 128 to 159 (U+0080 to U+009F) are not allowed in HTML;
even if they were, they would likely be unprintable control characters.
Tidy assumed you wanted to refer to a character with the same byte value in the
specified encoding and replaced that reference with the Unicode
equivalent.

...

First Tidy rejects both 0x96 characters, with a replacement warning, but outputs them differently! In the comment it is 0x13, UGH!,and in the paragraph is –. This is Tidy 5.5.76. In the tidy generated from issue-643 that paragraph output would be –...

So one question raised by the fact that the file passess the validator, "Is Tidy wrong to reject characters in the range 128 to 159?". Is that big info blurb it outputs wrong in principle?

That becomes very important to this issue.

If tidy accepted this 0x96, and others in this range, as a valid latin1, or windows-1252, or iso-8859-1, which ever is the best name for this character set, which in itself is another interesting question, then it would not be doing this substitution to 0x2013, or others, and we would not get 0x13, or something else bad, output to the document. Part of the problem solved...

And concerning the name of the character set, when I used the iso-8859-1 in the document, the validator suggested windows-1252 as a better name. Sort of totally at odds with tidy... but maybe that could be a new separate issue...

But if tidy is to continue to consider 128 to 159 as invalid then it seems the same substitution encoding should be done in the comment, as is done in the paragraph... that is, should at least be consistent... will try to do that meantime...

Tried --input-encoding latin1, --output-encoding utf-8. In a quick test, it was output as E2 80 93 in both cases... Of course, this still shows up as rubbish in my console, but any reasonable editor, even simple notepad, shows this as a dash... so maybe this is ok?

Still to do some more testing, poking and pondering...

Hope you, especially in the macOS, and others, anywhere, get a chance to try my sample, and provide more feedback... thanks...

[balthisar]

I've always hated dealing with character sets when the operating system doesn't handle the details itself! In my own LibTidy client, I keep LibTidy in pure UTF8, and let the OS handle all these nasty details. Clearly, though, there's a lot of value in LibTidy being able to do this.

Some highlights from this W3C/WHATWG page, which is pertinent to HTML5 (which is what your samples are):

• "Authors must use the UTF-8 encoding and must use the ASCII case-insensitive "utf-8" label to identify it." It seems right away that for HTML5, the sample document is possibly not valid because it's not UTF-8. This raises a good question: should be drop support for non-UTF8 output of HTML5 documents? We could accept non-UTF8 as input, but output could be forced to UTF8. Personally, I prefer giving users control of their output encoding, and not going this route.

• "The table below lists all encodings and their labels user agents must support. User agents must not support any other encodings or labels." This means browsers, but clearly Tidy is a user agent, too. What I gather from the table is, "encodings" are "encodings," and "labels" are synonyms that cause the UA to use the encoding. So, in the case of "latin1", Tidy should recognize that "windows-1252" is what should be processed. Of course, now we might argue that Tidy should also recognize all of the other labels for this encoding, too, as meta charset strings.

• Of course, HTML4 doesn't use meta charset, so the sample file, if were were checking encoding labels, would have to use a different meta declaration.

• In this document W3C strongly indicates that a lot of the encodings we support shouldn't be used, particularly the east Asian languages.

Now, I'm wondering why your console is showing rubbish? Tidy should be successfully forcing your console to UTF8 these days. Do you still get rubbish in the Power Shell? As far as I know, the only rubbish in the standard CMD these days should be with Asian languages.

When I check your sample document as-is, nothing is wrong with the character set, only the warning that UTF8 should be used. When I change it to HTML4 (including the meta), it validates perfectly, even with the out of range characters.

The aha moment! That link indicates that "For any other encoding, visual checking is necessary." The validator, it seems, is not a charset validator. I think that Tidy has to obey the specifications of the charset, and not depend on the validator for charset validity.

tl;dr: @geoffmcl, I think Tidy is doing the right thing in indicating the invalid character.

[geoffmcl]

@balthisar really thanks for the additional feedback, and links. This was very useful, and in some cases educational...

I now see this as a simple problem that tidy does not encode the text output in a html comment. This leads to very bad value being added to the html output file, and should be addressed, and hopefully fixed. Will work on that, but first some other peripheral issues raised...

... Tidy is doing the right thing in indicating the invalid character.

Not so sure about this! Yes, I get that maybe we all should be using utf-8, but it seems that the validity of windows-1252 is being over-looked.

So to me the Info output by tidy namely - Character codes 128 to 159 (U+0080 to U+009F) are not allowed in HTML - is NOT exactly true!

I have not found a browser yet - tried chrome, IE, Firefox, Opera, Safari, in Windows - and ALL correctly display this table temptable.html...

Yes, there are about 5 values that do not compute, 0x81, 0x8D, etc, marked with 'Name' N/A...

Of course if you pass this to the validator, then it will abort on detecting the raw 0x81...

If you remove these 5 or so, as in temptable2.html the validator will run, and even though the document clearly indicates it is windows-1252 charset, flags an error on many of the numeric character references, saying like A numeric character reference expanded to the C1 controls range..

But this is windows-1252, so they are not ISO 8859-1 C1 controls! This is not strictly an error...

And if you remove those numeric character references it is offended by, temptable3.html, and uncheck the Legacy encoding warning, will report Document checking completed. No errors or warnings to show....

This is all on a HTML 5 document, and as you suggest am sure will have similar results using a HTML 4 doctype, with the appropriate meta change...

All I am trying hard to suggest here, is to not go overboard on a windows-1252 charset ;=))

... we might argue that Tidy should also recognize all of the other labels for this encoding ...

Agreed, but see that as a separate, different issue...

Tidy has a much improved validator and even change, or add the appropriate meta charset - see #456, PR #565, and maybe others - of course this could all be improved, enhanced,... especially to recognize all labels... but again perhaps this is a separate issue...

Tidy should be successfully forcing your console to UTF8 these days.

As reported several times now, no this does not work in my console, nor in the Power Shell...

See this small image utf-8.jpg. The top blue is PS, and the bottom black is standard cmd... In both cases I see what some would call rubbish!

As mentioned before, I have tried many modifications of the cmd prompt, and have never seen utf-8 correctly displayed...

Of course it is valid utf-8 E2 80 93... That valid utf-8 sequence will show as a dash, -, in most editors... just not in my console...

In fact I would suggest removing this so called utf-8 enabling code from tidy.c. It does nothing, and would allow us to remove the <windows.h> include, and the setting up of an atexit service...

But is not that important, and is a separate issue anyway...

Main Issue Here

So back to the main issue addressed here, Tidy should not output 0x13 in a comment string. Such control values have no place in a html text file... and in my book is a BUG!

I am more or less satisfied, at least for now, that tidy does its current substitution of these values, and flags a warning, right or wrong!, and from my temptable*.html samples, the substituted entity looks correct... and this is what is output in all other html text...

So as stated, the only issue here is to avoid the real problem in html comment output... will try to work on that only... any further feedback appreciated... thanks...