Fix broken HTML entities in html output

[Sp1l]

• Output is missing the & in the html entities leading to incorrect display.

The --html flag currently produces html output with broken HTML entities

<span style="color:white;background-color:black;"> Start 2023-01-31 10:40:32        -->gt;>gt; 192.0.2.123:443 (example.com) <lt;<lt;--</span>

The patch fixes this for me

<span style="color:white;background-color:black;"> Start 2023-01-31 10:51:37        --&gt;&gt; 192.0.2.123:443 (example.com) &lt;&lt;--</span>

[dcooper16]

Hi @Sp1l,

Thanks for the PR. I tried it out on two systems: Ubuntu 20.04 and MacOS Monterey. While the PR works okay on Ubuntu, it produces incorrect results on MacOS. On MacOS, I get "\&gt" in the HTML rather than "&gt". On the other hand, the current code works for me on both systems.

So, it seems that the PR needs to be modified so that it fixes the HTML on the system you are using, but doesn't break things on other systems (e.g., MacOS). Could you provide some information about the system you are using on which the broken HTML is being created (e.g., uname -a, bash --version)?

Thanks,

David

[drwetter]

Wasn't there a CI check for MacOS too?

[dcooper16]

I can't find any that the CI checks are performed on MacOS. Looking at .github/workflows/test.yml I only see Ubuntu 20.04 listed. Should I be looking somewhere else?

In doing a search for "MacOS" I came across PR #1481 (and #1528 for the 3.0 branch). I seems that html_reserved() was originally written the way that this PR proposes, but I changed it because it wasn't working on MacOS.

[Sp1l]

Hi David,

So, it seems that the PR needs to be modified so that it fixes the HTML on the system you are using, but doesn't break things on other systems (e.g., MacOS). Could you provide some information about the system you are using on which the broken HTML is being created (e.g., uname -a, bash --version)?

This is where I admit not having read the contributing doc...

Basically stock Ubuntu 22.10 on WSL2, tend to use the PeterMosmans' SSL (I maintain the port for this on FreeBSD security/openssl-unsafe)

❯ uname -a
Linux nly101816-t14 5.15.79.1-microsoft-standard-WSL2 #1 SMP Wed Nov 23 01:01:46 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
❯ grep PRETTY_NAME /etc/os-release
PRETTY_NAME="Ubuntu 22.10"
❯ bash --version
GNU bash, version 5.2.2(1)-release (x86_64-pc-linux-gnu)
❯ echo $LANG
C.UTF-8

Is there any relation to sed which is used later in the emphasize_stuff_in_headers function? Ubuntu has GNU sed 4.8 which is not the same as BSD sed (does OSX have BSD sed or GNU sed?)
Just checked on FreeBSD and got the same -->gt;>gt; issue.

❯ grep PRETTY_NAME /var/run/os-release
PRETTY_NAME="FreeBSD 13.1-RELEASE"
❯ bash --version
GNU bash, version 5.2.15(0)-release (amd64-portbld-freebsd13.1)
❯ echo $LANG
en_US.UTF-8

Cheers, Bernard.

[dcooper16]

We've already encountered at least one problem (#1758) related to the use of bash 5.1. Perhaps this is another case of code that works with bash 5.0 and earlier, but not with bash 5.1 and later.

At the moment, I don't have a system that uses bash 5.1 or later. I'll see if I can get one set up at some point so that I can do some testing on this.

[dcooper16]

I tried out a live image of Kubuntu 22.10 and was able to reproduce the problem, but don't have a solution yet.

This may help though: https://unix.stackexchange.com/questions/733148/why-doesnt-ampersand-work-in-string-replacement-without-being-escaped.

It seems that issue is related to a change made in Bash 5.2. I tried the idea of putting the ampersand in quotes, and it worked in Ubuntu 20.04 and Ubuntu 22.10, but not in MacOS Monterey. On MacOS, the quotes were just included in the string, just as the stack exchange article said would happen with older versions of Bash.

I guess if we can't come up with any better solution, we can have an if ... then ... else that uses one version of the substitution commands or the other, depending on the value of $BASH_VERSINFO. It could be done in the same way that there are two versions of toupper() and tolower() defined for different versions of Bash.

[drwetter]

I guess if we can't come up with any better solution,

As you suggested : either parse the bash version. Alternatively that could be done providing a test, parse the result and then chose the proper replacement. I am afraid that the latter could be not easy to implement in a error-free fashion looking forward.

[dcooper16]

After looking through the Bash man page some more, I found something that seems to work; replacing

output="${output//</&lt;}"

output="${output//</$'&'lt;}"

I tried this on Ubuntu 20.04, Ubuntu 23.10, and MacOS Monterey, and it produces the correct results on all three. I don't understand exactly under what circumstances Bash treats $'&' the same as &. So, I'm relying more on the results of testing rather than a full understanding of the syntax.

I submitted PRs #2317 and #2318 for the 3.1dev and 3.0 branches, respectively, that implement this solution.

Assuming the CI checks pass for #2317 (as I expect), please let me know if you prefer this solution to having two different versions of html_reserved() that are selected from bash on Bash version number or the results of a test.

[drwetter]

lease let me know if you prefer this solution to having two different versions of html_reserved() that are selected from bash on Bash version number or the results of a test.

Thanks a lot David for your PRs! No, not at all. This is way better!

Thanks also @Sp1l for catching this and your suggested fix.

PS: CI checks on 3.0 are still kind of broken, see #2270 . So one has to look where exactly it fails