Skip to content

Address assay wizard CSP complaints #5071

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
labkey-adam merged 6 commits into from
Dec 21, 2023
Merged

Address assay wizard CSP complaints #5071

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ff7e816
Address assay wizard CSP complaints
labkey-adam Dec 20, 2023
6a84f2c
Restore close()
labkey-adam Dec 20, 2023
98064d6
try-with-resources & IOUtils
labkey-adam Dec 20, 2023
3e20979
Clean up radio button styling
labkey-adam Dec 20, 2023
68512c5
Address random warnings
labkey-adam Dec 20, 2023
8baab8f
Unchecked is not a thing
labkey-adam Dec 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions api/src/org/labkey/api/assay/AssayFileWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@
import java.util.Set;
import java.util.TreeMap;

/**
* User: jeckels
* Date: Sep 21, 2007
*/
public class AssayFileWriter<ContextType extends AssayRunUploadContext<? extends AssayProvider>>
{
private static final Logger LOG = LogManager.getLogger(AssayFileWriter.class);
Expand Down Expand Up @@ -176,7 +172,7 @@ public static File findUniqueFileName(String originalFilename, File dir)

public static Path findUniqueFileName(String originalFilename, Path dir)
{
if (originalFilename == null || "".equals(originalFilename))
if (originalFilename == null || originalFilename.isEmpty())
{
originalFilename = "[unnamed]";
}
Expand Down Expand Up @@ -250,9 +246,8 @@ public Map<String, File> savePostedFiles(ContextType context, Set<String> parame
{
Map<String, File> files = new TreeMap<>();
Set<String> originalFileNames = new HashSet<>();
if (context.getRequest() instanceof MultipartHttpServletRequest)
if (context.getRequest() instanceof MultipartHttpServletRequest multipartRequest)
{
MultipartHttpServletRequest multipartRequest = (MultipartHttpServletRequest)context.getRequest();
Iterator<Map.Entry<String, List<MultipartFile>>> iter = multipartRequest.getMultiFileMap().entrySet().iterator();
File dir = getFileTargetDir(context);
Deque<File> overflowFiles = new ArrayDeque<>(); // using a deque for easy removal of single elements
Expand Down Expand Up @@ -292,15 +287,15 @@ public Map<String, File> savePostedFiles(ContextType context, Set<String> parame
// now process overflow files, if any
for (String unusedParameterName : unusedParameterNames)
{
if (overflowFiles.size() < 1)
if (overflowFiles.isEmpty())
break; // we're done
else
{
files.put(unusedParameterName, overflowFiles.remove());
}
}

if (overflowFiles.size() > 0) // too many files; shouldn't happen, but if it does, throw an error
if (!overflowFiles.isEmpty()) // too many files; shouldn't happen, but if it does, throw an error
throw new ExperimentException("Tried to save too many files: number of keys is " + parameterNames.size() +
", but " + overflowFiles.size() + " extra file(s) were found.");
}
Expand Down
2 changes: 1 addition & 1 deletion api/src/org/labkey/api/reports/report/r/RReport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -820,7 +820,7 @@ public String getDownloadDataHelpMessage()
{
try
{
return new JspTemplate("/org/labkey/api/reports/report/view/rReportDesignerSyntaxRef.jsp").render();
return new JspTemplate<>("/org/labkey/api/reports/report/view/rReportDesignerSyntaxRef.jsp").render();
}
catch (Exception e)
{
Expand Down
1 change: 0 additions & 1 deletion api/src/org/labkey/api/util/element/DisplayField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import org.apache.commons.lang3.StringUtils;
import org.labkey.api.util.HtmlString;
import org.labkey.api.util.PageFlowUtil;

import java.io.IOException;

Expand Down
2 changes: 0 additions & 2 deletions api/src/org/labkey/api/view/JspTemplate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@

/**
* Executes a JSP and renders output to a string. Useful for JSP templating of SQL queries, etc.
* User: adam
* Date: Aug 10, 2010
*/
public class JspTemplate<ModelClass> extends JspView<ModelClass>
{
Expand Down
2 changes: 1 addition & 1 deletion api/src/org/labkey/api/view/template/PageConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ private void _addHandler(EventHandler eh)
* NOTE element.addListener(function) is not the same as element.onclick=function!
* This is for onevent handlers
*/
public void addHandler(String id, String event, String handler)
public void addHandler(String id, String event, @Nullable String handler)
{
if (StringUtils.isBlank(id) || StringUtils.isBlank(event))
throw new IllegalArgumentException();
Expand Down
63 changes: 23 additions & 40 deletions assay/api-src/org/labkey/api/assay/plate/PlateSampleFilePropertyHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.labkey.api.assay.plate;

import org.apache.commons.io.IOUtils;
import org.jetbrains.annotations.Nullable;
import org.labkey.api.assay.AssayDataCollector;
import org.labkey.api.assay.AssayFileWriter;
Expand All @@ -40,6 +41,7 @@
import org.labkey.api.util.PageFlowUtil;
import org.labkey.api.view.HttpView;
import org.labkey.api.view.InsertView;
import org.labkey.api.view.template.PageConfig;
import org.springframework.web.multipart.MultipartFile;
import org.springframework.web.multipart.MultipartHttpServletRequest;

Expand All @@ -57,24 +59,20 @@
import java.util.Objects;
import java.util.Set;

/**
* User: brittp
* Date: Aug 16, 2010 4:59:46 PM
*/
public class PlateSampleFilePropertyHelper extends PlateSamplePropertyHelper
{
private static final String SAMPLE_FILE_INPUT_NAME = "__sampleMetadataFile__";
private static final int BUFFER_SIZE = 2048;
public static final String SAMPLE_WELLGROUP_COLUMN = "SampleWellGroup";
public static final String PLATELOCATION_COLUMN = "PlateLocation";

private final Container _container;
private final SampleMetadataInputFormat _metadataInputFormat;

protected ExpProtocol _protocol;
protected Map<String, Map<DomainProperty, String>> _sampleProperties;
protected String _metadataNoun = "Sample";
protected String _wellGroupColumnName = SAMPLE_WELLGROUP_COLUMN;
private Container _container;
private File _metadataFile;
private SampleMetadataInputFormat _metadataInputFormat;

public PlateSampleFilePropertyHelper(Container container, ExpProtocol protocol, List<? extends DomainProperty> domainProperties, Plate template, SampleMetadataInputFormat inputFormat)
{
Expand All @@ -86,9 +84,8 @@ public PlateSampleFilePropertyHelper(Container container, ExpProtocol protocol,

private MultipartFile getMetadataFile(HttpServletRequest request)
{
if (request instanceof MultipartHttpServletRequest)
if (request instanceof MultipartHttpServletRequest multipartRequest)
{
MultipartHttpServletRequest multipartRequest = (MultipartHttpServletRequest) request;
String entryKeyToFind;
if (_metadataInputFormat == SampleMetadataInputFormat.FILE_BASED)
{
Expand Down Expand Up @@ -118,13 +115,10 @@ else if (_metadataInputFormat == SampleMetadataInputFormat.COMBINED)
@Override
public Map<String, Map<DomainProperty, String>> getPostedPropertyValues(HttpServletRequest request) throws ExperimentException
{
Map<String, Map<DomainProperty, String>> result = new HashMap<>();
Map<String, Map<DomainProperty, String>> sampleProperties = getSampleProperties(request);
if (sampleProperties == null || sampleProperties.isEmpty())
throw new ExperimentException(_metadataNoun + " metadata must be provided.");
for (Map.Entry<String, Map<DomainProperty, String>> entry : sampleProperties.entrySet())
result.put(entry.getKey(), entry.getValue());
return result;
return new HashMap<>(sampleProperties);
}

protected File getSampleMetadata(HttpServletRequest request) throws ExperimentException
Expand Down Expand Up @@ -153,28 +147,22 @@ protected File getSampleMetadata(HttpServletRequest request) throws ExperimentEx
MultipartFile metadata = getMetadataFile(request);
if (metadata == null)
return null;
BufferedOutputStream fos = null;
InputStream is = null;

try
{
File uploadDirectory = AssayFileWriter.ensureUploadDirectory(_container);
_metadataFile = AssayFileWriter.findUniqueFileName(metadata.getOriginalFilename(), uploadDirectory);
fos = new BufferedOutputStream(new FileOutputStream(_metadataFile));
is = metadata.getInputStream();
byte[] data = new byte[BUFFER_SIZE];
int bytes;
while ((bytes = is.read(data, 0, BUFFER_SIZE)) != -1)
fos.write(data, 0, bytes);
try (BufferedOutputStream fos = new BufferedOutputStream(new FileOutputStream(_metadataFile));
InputStream is = metadata.getInputStream())
{
IOUtils.copy(is, fos);
}
}
catch (IOException e)
{
throw new RuntimeException(e);
}
finally
{
try { if (fos != null) fos.close(); } catch (IOException ignored) {}
try { if (is != null) is.close(); } catch (IOException ignored) {}
}

return _metadataFile;
}

Expand Down Expand Up @@ -238,12 +226,7 @@ public Map<String, Map<DomainProperty, String>> getSampleProperties(HttpServletR

validateMetadataRow(row, wellGroupName, wellgroup);

Map<DomainProperty, String> sampleProperties = allProperties.get(wellgroup.getName());
if (sampleProperties == null)
{
sampleProperties = new HashMap<>();
allProperties.put(wellgroup.getName(), sampleProperties);
}
Map<DomainProperty, String> sampleProperties = allProperties.computeIfAbsent(wellgroup.getName(), k -> new HashMap<>());

for (DomainProperty property : _domainProperties)
{
Expand All @@ -260,7 +243,7 @@ public Map<String, Map<DomainProperty, String>> getSampleProperties(HttpServletR
}
catch (IOException e)
{
throw new ExperimentException("Unable to parse sample properties file. Please verify that the file is a valid TSV, CSV or Excel file.", e);
throw new ExperimentException("Unable to parse sample properties file. Please verify that the file is a valid TSV, CSV or Excel file.", e);
}
_sampleProperties = allProperties;
return _sampleProperties;
Expand All @@ -273,7 +256,7 @@ protected void validateMetadataRow(Map<String, Object> row, String wellGroupName
{
throw new ExperimentException("Well group \"" + wellGroupName + "\" is listed in plate location " +
plateLocation + ", but the stored plate template indicates that this group should be in location " +
wellgroup.getPositionDescription() + ". Please contact an administrator to correct the saved template " +
wellgroup.getPositionDescription() + ". Please contact an administrator to correct the saved template " +
"if sample locations have changed on the plate.");
}
}
Expand Down Expand Up @@ -364,6 +347,7 @@ public void renderInputHtml(RenderContext ctx, Writer out, Object value) throws
out.write("<br>");
if (reshowFile != null)
{
PageConfig pageConfig = HttpView.currentPageConfig();
PipeRoot pipelineRoot = PipelineService.get().findPipelineRoot(ctx.getContainer());
String filePath = PageFlowUtil.filter(pipelineRoot.relativePath(reshowFile).replace('\\', '/'));
String updateInputFn = "<script type=\"text/javascript\" nonce=\"" + HttpView.currentPageConfig().getScriptNonce() + "\">" +
Expand All @@ -374,18 +358,18 @@ public void renderInputHtml(RenderContext ctx, Writer out, Object value) throws
"</script>";
out.write(updateInputFn);
out.write("\n<table><tr>");
out.write("\n<td><input type=\"radio\" name=\"" + METADATA_PROVIDER_INPUT_NAME + "\" value=\"" +
METADATA_PROVIDER_OPTION_PREVUPLOAD + "\" onChange=\"showMetadataPicker(!this.checked);\" CHECKED></td>");
out.write("\n<td><input type=\"radio\" id=\"optionPrevUpload\" name=\"" + METADATA_PROVIDER_INPUT_NAME + "\" value=\"" + METADATA_PROVIDER_OPTION_PREVUPLOAD + "\" checked></td>");
pageConfig.addHandler("optionPrevUpload", "change", "showMetadataPicker(!this.checked);");
out.write("\n<td>Use the metadata that was already uploaded to the server</td>");
out.write("\n</tr><tr>");
out.write("\n<td></td><td><div id=\"previousMetadataFileName\" style=\"display:block\">" + PageFlowUtil.filter(reshowFile.getName()) + "</div></td>");
out.write("\n</tr><tr>");
out.write("\n<td><input type=\"hidden\" name=\"" + METADATA_PREVUPLOAD_LOCATION + "\" value=\"" + filePath + "\">");
out.write("\n<input type=\"radio\" name=\"" + METADATA_PROVIDER_INPUT_NAME + "\" value=\"" +
METADATA_PROVIDER_OPTION_NEWUPLOAD + "\" onChange=\"showMetadataPicker(this.checked);\"></td>");
out.write("\n<input type=\"radio\" id=\"optionNewUpload\" name=\"" + METADATA_PROVIDER_INPUT_NAME + "\" value=\"" + METADATA_PROVIDER_OPTION_NEWUPLOAD + "\"></td>");
pageConfig.addHandler("optionNewUpload", "change", "showMetadataPicker(this.checked);");
out.write("\n<td>Upload a data file</td>");
out.write("\n</tr><tr>");
out.write("\n<td></td><td><div id=\"newMetadataFileName\" style=\"display:none\"><input type=\"file\" id=\"" +
out.write("\n<td></td><td><div id=\"newMetadataFileName\" style=\"display:none\"><input type=\"file\" id=\"" +
SAMPLE_FILE_INPUT_NAME + "\" name=\"" + SAMPLE_FILE_INPUT_NAME + "\" size=\"40\" style=\"border: none\"></div></td>");
out.write("\n</tr></table>");
}
Expand All @@ -399,7 +383,6 @@ public void renderInputHtml(RenderContext ctx, Writer out, Object value) throws
}
}
});

}

public SampleMetadataInputFormat getMetadataInputFormat()
Expand Down
2 changes: 1 addition & 1 deletion core/resources/views/login.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<div class="auth-header auth-item">Terms of Use</div>
<div class="toucontent auth-item termsOfUseContent"></div>
<div class="auth-item">
<input type="checkbox" tabindex="4" name="approvedTermsOfUse" id="approvedTermsOfUse" class="auth-item" unchecked>
<input type="checkbox" tabindex="4" name="approvedTermsOfUse" id="approvedTermsOfUse" class="auth-item">
<label for="approvedTermsOfUse">I agree to these terms</label>
</div>
</div>
Expand Down
10 changes: 4 additions & 6 deletions core/src/org/labkey/core/junit/runner.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,20 @@
* limitations under the License.
*/
%>
<%@ page import="org.labkey.api.util.PageFlowUtil" %>
<%@ page import="org.junit.runner.Description" %>
<%@ page import="org.junit.runner.Request" %>
<%@ page import="org.junit.runner.Runner" %>
<%@ page import="org.labkey.api.view.ActionURL" %>
<%@ page import="org.labkey.api.view.HttpView" %>
<%@ page import="org.labkey.api.view.JspView" %>
<%@ page import="org.labkey.core.junit.JunitController" %>
<%@ page import="org.labkey.core.junit.JunitController.JUnitViewBean" %>
<%@ page import="org.labkey.core.junit.JunitController.RunAction" %>
<%@ page import="org.labkey.core.junit.JunitController.Run2Action" %>
<%@ page import="org.labkey.core.junit.JunitController.Run3Action" %>
<%@ page import="org.labkey.core.junit.JunitController.RunAction" %>
<%@ page import="static org.labkey.api.util.DOM.*" %>
<%@ page import="static org.labkey.api.util.DOM.Attribute.*" %>
<%@ page import="static org.labkey.api.util.HtmlString.NBSP" %>
<%@ page import="org.junit.runner.Request" %>
<%@ page import="org.junit.runner.Runner" %>
<%@ page import="org.junit.runner.Description" %>
<%@ page import="org.w3c.dom.css.CSS2Properties" %>
<%@ page extends="org.labkey.api.jsp.JspBase" %>
<%
JspView<JUnitViewBean> me = (JspView<JUnitViewBean>) HttpView.currentView();
Expand Down
2 changes: 1 addition & 1 deletion pipeline/src/org/labkey/pipeline/api/PipelineQuerySchema.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public FieldKey getContainerFieldKey()
folderColumn.setFk(new ContainerForeignKey(this));
folderColumn.setDisplayColumnFactory(ContainerDisplayColumn.FACTORY);
table.addColumn(folderColumn);
String urlExp = "/pipeline-status/details.view?rowId=${rowId}";
String urlExp = "/pipeline-status-details.view?rowId=${rowId}";
table.setDetailsURL(DetailsURL.fromString(urlExp));
table.setDescription("Contains one row per pipeline job");

Expand Down
6 changes: 3 additions & 3 deletions study/src/org/labkey/study/view/createStudy.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@
<tr>
<td class="lk-study-property">Timepoint Style<%=helpPopup("Timepoint Styles", "<p>When using visits, administrators assign a label and a range of numerical \"Sequence Numbers\" that are grouped into visits.</p><p>If using dates, data can be grouped by day or week.</p>", true)%></td>
<td align="left">
<labkey:input type="radio" name="timepointType" id="dateTimepointType" value="<%=TimepointType.DATE%>" checked="<%=(form.getTimepointType() == TimepointType.DATE)%>" onChange="document.getElementById('defaultDurationRow').style.display = document.getElementById('dateTimepointType').checked ? 'table-row' : 'none'; document.getElementById('defaultDateRow').style.display = document.getElementById('continuousTimepointType').checked ? 'none' : 'table-row';" /> Dates &nbsp;&nbsp;
<labkey:input type="radio" name="timepointType" value="<%=TimepointType.VISIT%>" checked="<%=(form.getTimepointType() == TimepointType.VISIT || form.getTimepointType() == null)%>" onChange="document.getElementById('defaultDurationRow').style.display = document.getElementById('dateTimepointType').checked ? 'table-row' : 'none'; document.getElementById('defaultDateRow').style.display = document.getElementById('continuousTimepointType').checked ? 'none' : 'table-row';" /> Assigned Visits &nbsp;&nbsp;
<labkey:input type="radio" name="timepointType" id="continuousTimepointType" value="<%=TimepointType.CONTINUOUS%>" checked="<%=(form.getTimepointType() == TimepointType.CONTINUOUS)%>" onChange="document.getElementById('defaultDurationRow').style.display = document.getElementById('dateTimepointType').checked ? 'table-row' : 'none'; document.getElementById('defaultDateRow').style.display = document.getElementById('continuousTimepointType').checked ? 'none' : 'table-row';" /> Continuous
<labkey:input type="radio" name="timepointType" formGroup="false" id="dateTimepointType" value="<%=TimepointType.DATE%>" checked="<%=(form.getTimepointType() == TimepointType.DATE)%>" onChange="document.getElementById('defaultDurationRow').style.display = document.getElementById('dateTimepointType').checked ? 'table-row' : 'none'; document.getElementById('defaultDateRow').style.display = document.getElementById('continuousTimepointType').checked ? 'none' : 'table-row';" /> Dates &nbsp;&nbsp;
<labkey:input type="radio" name="timepointType" formGroup="false" value="<%=TimepointType.VISIT%>" checked="<%=(form.getTimepointType() == TimepointType.VISIT || form.getTimepointType() == null)%>" onChange="document.getElementById('defaultDurationRow').style.display = document.getElementById('dateTimepointType').checked ? 'table-row' : 'none'; document.getElementById('defaultDateRow').style.display = document.getElementById('continuousTimepointType').checked ? 'none' : 'table-row';" /> Assigned Visits &nbsp;&nbsp;
<labkey:input type="radio" name="timepointType" formGroup="false" id="continuousTimepointType" value="<%=TimepointType.CONTINUOUS%>" checked="<%=(form.getTimepointType() == TimepointType.CONTINUOUS)%>" onChange="document.getElementById('defaultDurationRow').style.display = document.getElementById('dateTimepointType').checked ? 'table-row' : 'none'; document.getElementById('defaultDateRow').style.display = document.getElementById('continuousTimepointType').checked ? 'none' : 'table-row';" /> Continuous
</td>
</tr>
<tr id="defaultDateRow" style="display: <%= text(form.getTimepointType() == TimepointType.CONTINUOUS ? "none" : "table-row") %>">
Expand Down