diff --git a/testresults/src/org/labkey/testresults/SendTestResultsEmail.java b/testresults/src/org/labkey/testresults/SendTestResultsEmail.java index f406ed23..2fac3463 100644 --- a/testresults/src/org/labkey/testresults/SendTestResultsEmail.java +++ b/testresults/src/org/labkey/testresults/SendTestResultsEmail.java @@ -223,7 +223,7 @@ public Pair getHTMLEmail(org.labkey.api.security.User from) .append("\n" + run.getDuration() + (run.getHang() != null ? " (hang)" : "") + "") .append("\n" + run.getFailedtests() + "") .append("\n" + run.getLeaks().length + "") - .append("\n " + run.getGitHash() + "") + .append("\n " + PageFlowUtil.filter(run.getGitHash()) + "") .append(""); } } @@ -237,7 +237,7 @@ public Pair getHTMLEmail(org.labkey.api.security.User from) for (User u : missingUsers) { message.append("") - .append("\nMissing " + u.getUsername() + "") + .append("\nMissing " + PageFlowUtil.filter(u.getUsername()) + "") .append(""); } message.append("") @@ -267,13 +267,13 @@ public Pair getHTMLEmail(org.labkey.api.security.User from) ""); RunDetail[] problemRuns = problems.getRuns(); for (RunDetail run : problemRuns) - message.append("\n" + run.getUserName() + ""); + message.append("\n" + PageFlowUtil.filter(run.getUserName()) + ""); message.append("\n"); for (String test : problems.getTestNames()) { message.append("\n") - .append("\n" + test + ""); + .append("\n" + PageFlowUtil.filter(test) + ""); for (RunDetail run : problemRuns) { message.append("\n"); diff --git a/testresults/src/org/labkey/testresults/TestResultsController.java b/testresults/src/org/labkey/testresults/TestResultsController.java index 5b99149a..f35bfd32 100644 --- a/testresults/src/org/labkey/testresults/TestResultsController.java +++ b/testresults/src/org/labkey/testresults/TestResultsController.java @@ -68,7 +68,6 @@ import org.labkey.api.view.JspView; import org.labkey.api.view.NavTree; import org.labkey.api.view.WebPartView; -import org.labkey.testresults.model.GlobalSettings; import org.labkey.testresults.model.RunDetail; import org.labkey.testresults.model.TestFailDetail; import org.labkey.testresults.model.TestHandleLeakDetail; @@ -450,7 +449,7 @@ public static void ensureRunDataCached(RunDetail[] runs, boolean keepObjData) { } catch (IOException e) { - e.printStackTrace(); + _log.error("Failed to encode run pass summary", e); } int avgMem = 0; if (passes.length != 0) @@ -1173,7 +1172,7 @@ public void setFlag(Boolean flag) /** * action to show all flagged runs flagged.jsp */ - @RequiresNoPermission + @RequiresPermission(ReadPermission.class) public static class ShowFlaggedAction extends SimpleViewAction { @Override @@ -1228,23 +1227,26 @@ public Object execute(BoundariesForm form, BindException errors) throws Exceptio return new ApiSimpleResponse(res); } - GlobalSettings settings = new GlobalSettings(warningB, errorB); - DbScope.Transaction transaction = TestResultsSchema.getSchema().getScope().ensureTransaction(); - SQLFragment sqlFragment = new SQLFragment(); - sqlFragment.append("select exists(select 1 from " + TestResultsSchema.getTableInfoGlobalSettings() + ") "); - SqlSelector sqlSelector = new SqlSelector(TestResultsSchema.getSchema(), sqlFragment); - List values = new ArrayList<>(); - sqlSelector.forEach(rs -> values.add(rs.getBoolean(1))); - - if (values.get(0)) { - SQLFragment sqlFragmentDelete = new SQLFragment(); - sqlFragmentDelete.append("DELETE FROM " + TestResultsSchema.getTableInfoGlobalSettings()); - new SqlExecutor(TestResultsSchema.getSchema()).execute(sqlFragmentDelete); + try (DbScope.Transaction transaction = TestResultsSchema.getSchema().getScope().ensureTransaction()) + { + SQLFragment sqlFragment = new SQLFragment(); + sqlFragment.append("select exists(select 1 from " + TestResultsSchema.getTableInfoGlobalSettings() + ") "); + SqlSelector sqlSelector = new SqlSelector(TestResultsSchema.getSchema(), sqlFragment); + List values = new ArrayList<>(); + sqlSelector.forEach(rs -> values.add(rs.getBoolean(1))); + + if (values.get(0)) { + SQLFragment sqlFragmentDelete = new SQLFragment(); + sqlFragmentDelete.append("DELETE FROM " + TestResultsSchema.getTableInfoGlobalSettings()); + new SqlExecutor(TestResultsSchema.getSchema()).execute(sqlFragmentDelete); + } + SQLFragment sqlFragmentInsert = new SQLFragment(); + sqlFragmentInsert.append("INSERT INTO " + TestResultsSchema.getTableInfoGlobalSettings() + " (warningb, errorb) VALUES (?, ?)"); + sqlFragmentInsert.add(warningB); + sqlFragmentInsert.add(errorB); + new SqlExecutor(TestResultsSchema.getSchema()).execute(sqlFragmentInsert); + transaction.commit(); } - SQLFragment sqlFragmentInsert = new SQLFragment(); - sqlFragmentInsert.append("INSERT INTO " + TestResultsSchema.getTableInfoGlobalSettings() + " (warningb, errorb) VALUES (" + warningB + ", " + errorB +")"); - new SqlExecutor(TestResultsSchema.getSchema()).execute(sqlFragmentInsert); - transaction.commit(); res.put("Message", "success!"); return new ApiSimpleResponse(res); } @@ -1779,6 +1781,8 @@ else if (xml.isEmpty()) _log.info("Attempting to save file for a future post attempt"); res.put(KEY_SUCCESS, false); res.put("Message", "Error Parsing XML attempting to save the XML file... " + NIGHTLY_POSTER.SaveXML(file, getContainer())); + // The stack trace is intentionally returned to the caller (SkylineTester) so posting + // failures can be diagnosed from the client without server log access. res.put("Exception", e + NIGHTLY_POSTER.getStackTraceText(e)); return new ApiSimpleResponse(res); } @@ -1848,6 +1852,8 @@ public Object execute(Object o, BindException errors) f.delete(); res.put(f.getName(), "Success!"); } catch (Exception e) { + // The stack trace is intentionally returned to the caller so a failed re-post can be + // diagnosed from the client without server log access. res.put(f.getName(), Arrays.toString(e.getStackTrace())); } } @@ -1890,15 +1896,14 @@ private static String SaveXML(MultipartFile file, Container c) { { File f = makeFile(c, fileName); if(f.exists()) { - _log.info("A file by the name " + fileName + " is already stored."); + _log.info("A file by the name {} is already stored.", fileName); return "File not saved - file already exists in file system."; } file.transferTo(f); } catch (IOException e) { - _log.error("Failed to save " + fileName + "."); - e.printStackTrace(); + _log.error("Failed to save {}.", fileName, e); return "Failed to save the file."; } return "File saved to system."; diff --git a/testresults/src/org/labkey/testresults/TestResultsModule.java b/testresults/src/org/labkey/testresults/TestResultsModule.java index bdd474ec..50af20ad 100644 --- a/testresults/src/org/labkey/testresults/TestResultsModule.java +++ b/testresults/src/org/labkey/testresults/TestResultsModule.java @@ -16,6 +16,7 @@ package org.labkey.testresults; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.data.Container; @@ -24,6 +25,7 @@ import org.labkey.api.module.ModuleContext; import org.labkey.api.security.Directive; import org.labkey.api.security.SecurityManager; +import org.labkey.api.util.logging.LogHelper; import org.labkey.filters.ContentSecurityPolicyFilter; import org.labkey.api.view.WebPartFactory; import org.quartz.JobKey; @@ -42,6 +44,8 @@ public class TestResultsModule extends DefaultModule { + private static final Logger LOG = LogHelper.getLogger(TestResultsModule.class, "Test results module"); + public static final WebPartFactory _testResultsFactory = new TestResultsWebPart(); public static final String JOB_NAME = "TestResultsEmailTrigger"; public static final String JOB_GROUP = "TestResultsGroup"; @@ -127,7 +131,7 @@ public void startBackgroundThreads() } catch (SchedulerException e) { - e.printStackTrace(); + LOG.error("Failed to start the test results email scheduler", e); } } diff --git a/testresults/src/org/labkey/testresults/TestResultsWebPart.java b/testresults/src/org/labkey/testresults/TestResultsWebPart.java index 2a8a9a53..95878a89 100644 --- a/testresults/src/org/labkey/testresults/TestResultsWebPart.java +++ b/testresults/src/org/labkey/testresults/TestResultsWebPart.java @@ -1,7 +1,9 @@ package org.labkey.testresults; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.labkey.api.data.Container; +import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.BaseWebPartFactory; import org.labkey.api.view.JspView; import org.labkey.api.view.Portal; @@ -16,6 +18,8 @@ public class TestResultsWebPart extends BaseWebPartFactory { + private static final Logger LOG = LogHelper.getLogger(TestResultsWebPart.class, "Test results web part"); + public TestResultsWebPart() { super("Test Results", true, false, WebPartFactory.LOCATION_BODY); @@ -32,7 +36,7 @@ public WebPartView getWebPartView(@NotNull ViewContext portalCtx, Portal.@Not } catch (ParseException | IOException e) { - e.printStackTrace(); + LOG.error("Failed to build the test results web part data", e); } JspView view = new JspView<>("/org/labkey/testresults/view/rundown.jsp", bean); view.setTitle("Test Results"); diff --git a/testresults/src/org/labkey/testresults/model/RunDetail.java b/testresults/src/org/labkey/testresults/model/RunDetail.java index 8202cdac..83c5cbae 100644 --- a/testresults/src/org/labkey/testresults/model/RunDetail.java +++ b/testresults/src/org/labkey/testresults/model/RunDetail.java @@ -15,8 +15,10 @@ */ package org.labkey.testresults.model; +import org.apache.logging.log4j.Logger; import org.labkey.api.data.Container; import org.labkey.api.reader.Readers; +import org.labkey.api.util.logging.LogHelper; import java.io.BufferedReader; import java.io.ByteArrayInputStream; @@ -34,6 +36,8 @@ */ public class RunDetail implements Comparable { + private static final Logger LOG = LogHelper.getLogger(RunDetail.class, "Test results run detail"); + public final static int HANG_MILLISECONDS = 30*60*1000; // 30 minutes private int id; @@ -275,7 +279,7 @@ public static String decode(byte[] bytes) { } catch (IOException e1) { - e1.printStackTrace(); + LOG.error("Error reading gzipped run log", e1); } return out.toString(); } diff --git a/testresults/src/org/labkey/testresults/view/failureDetail.jsp b/testresults/src/org/labkey/testresults/view/failureDetail.jsp index b334fd3d..508f4b40 100644 --- a/testresults/src/org/labkey/testresults/view/failureDetail.jsp +++ b/testresults/src/org/labkey/testresults/view/failureDetail.jsp @@ -333,8 +333,8 @@ $(document).ready(function() { headerName = "Failures"; displayFunc = run => { return "
    " + run.failures.map(f => "
  • " + - "Pass " + f.pass.toString() + " (" + f.language + ") ---" + - '
    ' + f.trace + "
    "+ + "Pass " + f.pass.toString() + " (" + LABKEY.Utils.encodeHtml(f.language) + ") ---" + + '
    ' + LABKEY.Utils.encodeHtml(f.trace) + "
    "+ "
  • ").join() + "
"; }; jsonKey = "failures"; diff --git a/testresults/src/org/labkey/testresults/view/runDetail.jsp b/testresults/src/org/labkey/testresults/view/runDetail.jsp index c4895a38..b1c39187 100644 --- a/testresults/src/org/labkey/testresults/view/runDetail.jsp +++ b/testresults/src/org/labkey/testresults/view/runDetail.jsp @@ -46,7 +46,11 @@ var win = window.open("", "Log file", "toolbar=no,location=no,directories=no,status=no,menubar=no,scrollbars=yes,resizable=yes," + "width=800,height=600"); - win.document.write('
' + data + '
'); + // The log/xml is stored run content. Set it as text rather than writing it as HTML so it cannot execute as + // markup in the popup. + var pre = win.document.createElement('pre'); + pre.textContent = data; + win.document.body.appendChild(pre); }; var showLog = function() { $.get('<%=h(new ActionURL(TestResultsController.ViewLogAction.class, c).addParameter("runId", runId))%>', csrf_header, diff --git a/testresults/src/org/labkey/testresults/view/trainingdata.jsp b/testresults/src/org/labkey/testresults/view/trainingdata.jsp index 69d4d5cd..86390b17 100644 --- a/testresults/src/org/labkey/testresults/view/trainingdata.jsp +++ b/testresults/src/org/labkey/testresults/view/trainingdata.jsp @@ -288,7 +288,16 @@ var win = window.open("", data.subject, "toolbar=no,location=no,directories=no,status=no,menubar=no,scrollbars=yes,resizable=yes," + "width=800,height=600"); - win.document.write(data.HTML); + // data.HTML is a rendered email containing stored run/user/test names. Render it in a sandboxed iframe + // (no allow-scripts, opaque origin) so any markup it contains is displayed but cannot execute as script. + var iframe = win.document.createElement('iframe'); + iframe.setAttribute('sandbox', ''); + // Use viewport height: a percentage height would resolve against the popup body + // (which has no defined height) and collapse the iframe to zero, showing nothing. + iframe.style.cssText = 'border:0;width:100%;height:100vh;display:block;'; + iframe.srcdoc = data.HTML; + win.document.body.style.margin = '0'; + win.document.body.appendChild(iframe); }, "json") }); diff --git a/testresults/test/src/org/labkey/test/tests/testresults/TestResultsTest.java b/testresults/test/src/org/labkey/test/tests/testresults/TestResultsTest.java index 1b1c37b9..720710a2 100644 --- a/testresults/test/src/org/labkey/test/tests/testresults/TestResultsTest.java +++ b/testresults/test/src/org/labkey/test/tests/testresults/TestResultsTest.java @@ -516,6 +516,28 @@ public void testViewXml() assertFalse("XML should not contain Log element (stripped before storage)", xmlContent.contains("")); } + @Test + public void testViewLogPopup() + { + // Verify the popup actually displays the stored log. + navigateToRunById(_cleanRunId); + click(Locator.lkButton("View Log")); + switchToWindow(1); + try + { + waitForElement(Locator.tag("pre")); + String popupText = getText(Locator.tag("pre")); + assertTrue("Log popup should show the nightly header, was: " + popupText, + popupText.contains("# Nightly started Thursday, January 15, 2026 9:00 PM")); + assertTrue("Log popup should show test entries", popupText.contains("TestAlpha")); + } + finally + { + closeExtraWindows(); + switchToMainWindow(); + } + } + @Test public void testChangeBoundaries() {