Fix #8542 - Escape HTML characters. #8544

jmarrec · 2021-02-17T15:36:35Z

Pull request overview

Fixes Some IDF object names can break the HTML output file #8542 - Some IDF object names can break the HTML output file
This PR escapes html characters when outputing to HTML.

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

Reviewer

This will not be exhaustively relevant to every PR.

Added an optional boolean argument to ConvertToEscape to not convert " and ' like it would in the original routine which was meant for XML. This is to support HTML4 mostly, but might as well be correct.

jmarrec · 2021-02-17T15:37:14Z

Unit Test before at `0dcf25b`

/home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/OutputReportTabular.unit.cc:8483: Failure
Expected equality of these values:
  expectedHTMLString
    Which is: "<td align=\"right\">My Coil &lt;coil is DX&gt;</td>"
  found_cell
    Which is: "<td align=\"right\">My Coil <coil is DX></td>"
<td align="right">My Coil <coil is DX></td>
/home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/OutputReportTabular.unit.cc:8483: Failure
Expected equality of these values:
  expectedHTMLString
    Which is: "<td align=\"right\">My Design Day where it's &gt;= 8&deg;</td>"
  found_cell
    Which is: "<td align=\"right\">My Design Day where it's >= 8\xC2\xB0</td>"
    As Text: "<td align="right">My Design Day where it's >= 8°</td>"
<td align="right">My Design Day where it's >= 8°</td>

After it passes

jmarrec · 2021-02-17T15:40:13Z

I added a MCVE on https://github.com/NREL/EnergyPlusDevSupport/blob/c8f21351be86526a2970cf009d2f2309f6578846/DefectFiles/8000s/8542/mcve.idf#L7

Before:

After:

HTML diff:

Note: In both cases the CO2 table looks the same visually:

…d escaping as well

jmarrec · 2021-03-01T10:34:26Z

Merged develop and resolved conflicts

jmarrec · 2021-03-01T10:09:30Z

src/EnergyPlus/OutputReportTabular.cc

+        //               " ' < > & degree-sign
+        //   If isXML=false, it does an HTML conversion, so only ` < > & ` and degree sign
+        //   Technically HTML4 doesn't support &quot, though most browsers would anyways.
+        //   Also, escaping single and double quotes is only needed inside attributes


Mod ConvertToEscaped to add a bool isXML so it can be used for HTML escaping too, which is quite similar

jmarrec · 2021-03-01T10:35:12Z

src/EnergyPlus/OutputReportTabular.hh

+    std::string ConvertToEscaped(std::string const &inString,  // Input String
+                                 bool isXML=true);             // isXML if false assumes HTML and will not convert quotes and apostrophes, for HTML4


jmarrec · 2021-03-01T10:36:20Z

tst/EnergyPlus/unit/OutputReportTabular.unit.cc

+TEST_F(SQLiteFixture, OutputReportTabularTest_EscapeHTML)
+{
+    // Test for #8542 - Ensures strings are escaped before going to HTML
+    EnergyPlus::sqlite->sqliteBegin();
+    EnergyPlus::sqlite->createSQLiteSimulationsRecord(1, "EnergyPlus Version", "Current Time");
+
+    auto &ort(state->dataOutRptTab);
+    ort->numStyles = 1;
+    ort->TableStyle(1) = OutputReportTabular::iTableStyle::HTML;
+    ort->del(1) = DataStringGlobals::CharSpace; // space - this is not used much for HTML output
+
+    ort->WriteTabularFiles = true;
+
+    SetupUnitConversions(*state);
+    ort->unitsStyle = OutputReportTabular::iUnitsStyle::JtoKWH;
+
+    SetPredefinedTables(*state);
+    std::string CompName = "My Coil <coil is DX>";
+
+    PreDefTableEntry(*state, state->dataOutRptPredefined->pdchDXCoolCoilType, CompName, "Coil:Cooling:DX:SingleSpeed");
+    // This would normally be called with numerics such as CompName, 0.006, 8, but I don't really care
+    PreDefTableEntry(*state, state->dataOutRptPredefined->pdch2CoilLvgHumRatIdealPeak, CompName,  "My Design Day where it's >= 8\u00B0");
+    PreDefTableEntry(*state, state->dataOutRptPredefined->pdst2CoilSummaryCoilSelection, CompName,  "My Design Day where it's >= 8\u00B0"); // this is >= 8 degree sign
+
+    // We enable the reports we care about, making sure we have the right ones
+    EXPECT_EQ("HVACSizingSummary", state->dataOutRptPredefined->reportName(6).name);
+    state->dataOutRptPredefined->reportName(6).show = true;
+
+    OutputReportTabular::OpenOutputTabularFile(*state);
+
+    WritePredefinedTables(*state);
+
+    OutputReportTabular::CloseOutputTabularFile(*state);
+
+    std::vector<std::string> lines = read_lines_in_file(DataStringGlobals::outputTblHtmFileName);


Setup the unit test. We write some stuff to the HTML with special characters. Ensures the HTML is closed. Then we read back the html lines.

jmarrec · 2021-03-01T10:36:50Z

tst/EnergyPlus/unit/OutputReportTabular.unit.cc

+    // Lambda helper to locate a line in the html file, and compare that line with the expected html after trimming
+    auto compare_html_output = [this, &lines](const std::string& lookup, const std::string& expectedHTMLString) {
+        std::string found_cell;
+        for (const auto& line: lines) {
+            if (line.find(lookup) != std::string::npos) {
+                found_cell = line;
+                break;
+            }
+        }
+        EXPECT_FALSE(found_cell.empty())
+            << "Did not find the lookup string '" << lookup
+            << "' string in the html output at '" << DataStringGlobals::outputTblHtmFileName
+            << "'..." << '\n' << delimited_string(lines);
+
+        // Trim leading and trailing spaces
+        found_cell.erase(0, found_cell.find_first_not_of(' ')); // ltrim
+        found_cell.erase(found_cell.find_last_not_of(' ') + 1); // rtrim
+
+        EXPECT_EQ(expectedHTMLString, found_cell) << found_cell;
+    };


A lambda to check we can actually find something in the HTML lines.

jmarrec · 2021-03-01T10:37:35Z

tst/EnergyPlus/unit/OutputReportTabular.unit.cc

+        EXPECT_EQ(expectedHTMLString, found_cell) << found_cell;
+    };
+
+    compare_html_output("My Coil", "<td align=\"right\">My Coil &lt;coil is DX&gt;</td>");


First test. I can locate the line header for My Coil <coil is DX> and it is properly escaped.

jmarrec · 2021-03-01T10:38:53Z

tst/EnergyPlus/unit/OutputReportTabular.unit.cc

+    // Clean up
+    FileSystem::removeFile(DataStringGlobals::outputTblHtmFileName);


And we clean up to be a good citizen.

jmarrec · 2021-03-01T10:39:20Z

src/EnergyPlus/OutputReportTabular.cc

+                    tbl_stream << "<p>Building: <b>" << ConvertToEscaped(BuildingName, false) << "</b></p>\n";
                    if (state.dataEnvrn->EnvironmentName == state.dataEnvrn->WeatherFileLocationTitle) {
-                        tbl_stream << "<p>Environment: <b>" << state.dataEnvrn->EnvironmentName << "</b></p>\n";
+                        tbl_stream << "<p>Environment: <b>" << ConvertToEscaped(state.dataEnvrn->EnvironmentName, false) << "</b></p>\n";
                    } else {
-                        tbl_stream << "<p>Environment: <b>" << state.dataEnvrn->EnvironmentName << " ** " << state.dataEnvrn->WeatherFileLocationTitle << "</b></p>\n";
+                        tbl_stream << "<p>Environment: <b>" << ConvertToEscaped(state.dataEnvrn->EnvironmentName, false) << " ** " << ConvertToEscaped(state.dataEnvrn->WeatherFileLocationTitle, false) << "</b></p>\n";


Escape at the very top of the HTML file.

jmarrec · 2021-03-01T10:40:06Z

src/EnergyPlus/OutputReportTabular.cc

@@ -15331,7 +15331,7 @@ namespace EnergyPlus::OutputReportTabular {
                for (iCol = 1; iCol <= colsColumnLabels; ++iCol) {
                    outputLine = "    <td align=\"right\">";
                    for (jRow = 1; jRow <= maxNumColLabelRows; ++jRow) {
-                        outputLine += colLabelMulti(iCol, jRow);
+                        outputLine += ConvertToEscaped(colLabelMulti(iCol, jRow), false);


Escape column headers, isXML=False

jmarrec · 2021-03-01T10:40:28Z

src/EnergyPlus/OutputReportTabular.cc

@@ -15343,13 +15343,13 @@ namespace EnergyPlus::OutputReportTabular {
                for (jRow = 1; jRow <= rowsBody; ++jRow) {
                    tbl_stream << "  <tr>\n";
                    if (rowLabels(jRow) != "") {
-                        tbl_stream << "    <td align=\"right\">" << InsertCurrencySymbol(state, rowLabels(jRow), true) << "</td>\n";
+                        tbl_stream << "    <td align=\"right\">" << ConvertToEscaped(InsertCurrencySymbol(state, rowLabels(jRow), true), false) << "</td>\n";


Escape row headers

jmarrec · 2021-03-01T10:40:45Z

src/EnergyPlus/OutputReportTabular.cc

                    } else {
                        tbl_stream << "    <td align=\"right\">&nbsp;</td>\n";
                    }
                    for (iCol = 1; iCol <= colsBody; ++iCol) {
                        if (body(iCol, jRow) != "") {
-                            tbl_stream << "    <td align=\"right\">" << InsertCurrencySymbol(state, body(iCol, jRow), true) << "</td>\n";
+                            tbl_stream << "    <td align=\"right\">" << ConvertToEscaped(InsertCurrencySymbol(state, body(iCol, jRow), true), false) << "</td>\n";


Escape every regular table cell.

Myoldmopar · 2021-03-01T19:50:34Z

tst/EnergyPlus/unit/OutputReportTabular.unit.cc

+    // We ensure that SQL doesn't have the same escape
+    for (const std::string reportName: {"HVACSizingSummary"}) {
+
+
+        auto result = queryResult("SELECT RowName, Value From TabularDataWithStrings "
+                                 "WHERE ReportName = \"" + reportName + "\""
+                                 "  AND ColumnName = \"Coil Leaving Air Humidity Ratio at Ideal Loads Peak\"",
+                                 "TabularDataWithStrings");
+
+        EnergyPlus::sqlite->sqliteCommit();
+
+        EXPECT_EQ(1u, result.size());
+        // Because the table has 8 cols
+        EXPECT_EQ(8u, result[0].size());
+
+        // 0.006 is a ratio, so unitconv = 1
+        std::string s = result[0][0];
+        // Trim the string, it has leading spaces
+        //s.erase(std::remove_if(s.begin(), s.end(), ::isspace), s.end());
+
+        EXPECT_EQ("My Coil <coil is DX>", s);
+
+        EXPECT_EQ("My Design Day where it's >= 8\u00B0", result[0][1]);
+    }


Myoldmopar · 2021-03-01T20:12:35Z

OK, pulled develop and verified the html is not sanitized, and indeed, brackets are treated as child html elements and hidden away. With this branch it looks great. Thanks for the fix @jmarrec , merging.

jmarrec added 2 commits February 17, 2021 16:10

Add a test for #8542

0dcf25b

Fix #8542 - Escape HTML characters.

166feaf

Added an optional boolean argument to ConvertToEscape to not convert " and ' like it would in the original routine which was meant for XML. This is to support HTML4 mostly, but might as well be correct.

jmarrec added the Defect Includes code to repair a defect in EnergyPlus label Feb 17, 2021

jmarrec self-assigned this Feb 17, 2021

jmarrec mentioned this pull request Feb 17, 2021

Some IDF object names can break the HTML output file #8542

Closed

3 tasks

jmarrec added 2 commits February 17, 2021 16:45

Couple of extra things atop the file (like a "Header") that could nee…

7ac466d

…d escaping as well

Merge remote-tracking branch 'origin/develop' into 8542_EscapeHTML

3610327

jmarrec added the NotIDDChange Code does not impact IDD (can be merged after IO freeze) label Mar 1, 2021

jmarrec commented Mar 1, 2021

View reviewed changes

jmarrec requested a review from JasonGlazer March 1, 2021 10:41

jmarrec added this to the EnergyPlus 9.5.0 milestone Mar 1, 2021

Myoldmopar approved these changes Mar 1, 2021

View reviewed changes

Myoldmopar merged commit e5d85ad into develop Mar 1, 2021

Myoldmopar deleted the 8542_EscapeHTML branch March 1, 2021 20:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #8542 - Escape HTML characters. #8544

Fix #8542 - Escape HTML characters. #8544

jmarrec commented Feb 17, 2021 •

edited

Loading

jmarrec commented Feb 17, 2021

jmarrec commented Feb 17, 2021 •

edited

Loading

jmarrec commented Mar 1, 2021

jmarrec Mar 1, 2021

jmarrec Mar 1, 2021

jmarrec Mar 1, 2021

jmarrec Mar 1, 2021

jmarrec Mar 1, 2021

jmarrec Mar 1, 2021

jmarrec Mar 1, 2021

jmarrec Mar 1, 2021

jmarrec Mar 1, 2021

jmarrec Mar 1, 2021

Myoldmopar Mar 1, 2021

Myoldmopar commented Mar 1, 2021

		std::string ConvertToEscaped(std::string const &inString, // Input String
		bool isXML=true); // isXML if false assumes HTML and will not convert quotes and apostrophes, for HTML4

		// Clean up
		FileSystem::removeFile(DataStringGlobals::outputTblHtmFileName);

Fix #8542 - Escape HTML characters. #8544

Fix #8542 - Escape HTML characters. #8544

Conversation

jmarrec commented Feb 17, 2021 • edited Loading

Pull request overview

Pull Request Author

Reviewer

jmarrec commented Feb 17, 2021

Unit Test before at 0dcf25b

jmarrec commented Feb 17, 2021 • edited Loading

jmarrec commented Mar 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Myoldmopar commented Mar 1, 2021

jmarrec commented Feb 17, 2021 •

edited

Loading

Unit Test before at `0dcf25b`

jmarrec commented Feb 17, 2021 •

edited

Loading