Skip to content
This repository has been archived by the owner on Nov 13, 2023. It is now read-only.

Fix angle brackets escaped in web help code blocks #731

Merged
merged 2 commits into from
Jan 31, 2022

Conversation

t1m0thyj
Copy link
Member

@t1m0thyj t1m0thyj commented Jan 28, 2022

Both the markdown-it and sanitize-html packages used to generate web help should handle angle brackets correctly.

The problem seems to have been we were calling them in the wrong order - sanitizing an MD/HTML mix before passing it to markdown-it rather than sanitizing the output HTML resulted in some unexpected behavior.

While investigating this, I realized the amount of HTML tags being injected into the MD we feed to markdown-it was very small (only in the method buildChildrenSummaryTables). So this PR replaces those HTML tags with Markdown, removes the html: true option from the markdown-it parser, and no longer requires the sanitize-html package 🙂

Here is a diff showing the changes introduced by this branch to the all.html file in web help: web-help-730.patch

Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
Comment on lines -273 to -276
} else if (/^\s*#{4}\s*COMMANDS\s*$/i.test(line)) {
return `${(index > 0) ? "\n" : ""}<h4>Commands</h4>\n`;
} else if (/^\s*#{4}\s*GROUPS\s*$/i.test(line)) {
return `${(index > 0) ? "\n" : ""}<h4>Groups</h4>\n`;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "hack" is no longer needed, because I fixed the casing of "Commands" and "Groups" in DefaultHelpGenerator.

@@ -320,17 +315,13 @@ export class WebHelpGenerator {
{ commandDefinition: definition, fullCommandTree: this.mFullCommandTree, skipTextWrap: true });

let markdownContent = helpGen.buildHelp() + "\n";
markdownContent = markdownContent.replace(/</g, "&lt;").replace(/>/g, "&gt;");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "hack" is no longer needed, since markdown-it handles angle brackets correctly. It seems to have been left behind as a remnant from when we were using marked.

if (definition.type === "group") {
// this is disabled for the CLIReadme.md but we want to show children here
// so we'll call the help generator's children summary function even though
// it's usually skipped when producing markdown
markdownContent += this.buildChildrenSummaryTables(helpGen, rootCommandName + "_" + fullCommandName);
}

// Prevent line breaks from being lost during Markdown to HTML conversion
markdownContent = markdownContent.replace(/^(\s+Default value:.+$)(\s+Allowed values:.+$)/gm, "$1\n$2");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "hack" is no longer needed, thanks to zowe/zowe-cli#1275

@t1m0thyj t1m0thyj linked an issue Jan 28, 2022 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Jan 28, 2022

Codecov Report

Merging #731 (ab54db4) into master (39bfd32) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #731      +/-   ##
==========================================
- Coverage   82.53%   82.52%   -0.02%     
==========================================
  Files         163      163              
  Lines        7937     7931       -6     
  Branches     1403     1400       -3     
==========================================
- Hits         6551     6545       -6     
  Misses       1382     1382              
  Partials        4        4              
Impacted Files Coverage Δ
packages/cmd/src/help/DefaultHelpGenerator.ts 97.50% <100.00%> (+0.01%) ⬆️
packages/cmd/src/help/WebHelpGenerator.ts 95.55% <100.00%> (-0.22%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffb2181...ab54db4. Read the comment docs.

Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@t1m0thyj t1m0thyj merged commit f86200d into master Jan 31, 2022
@t1m0thyj t1m0thyj deleted the fix-web-help-escape branch January 31, 2022 22:01
@zFernand0 zFernand0 removed their request for review January 19, 2023 14:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Web help wrongly escapes characters inside code blocks
3 participants