From 5a07b8ed59d1c00fff937968ca7d65167e8196d9 Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 23 May 2025 00:00:05 -0700 Subject: [PATCH 1/6] Issue 52925: App export to csv/tsv ignores filter with column containing double quote --- api/src/org/labkey/api/query/QueryParam.java | 2 +- .../org/labkey/api/query/QuerySettings.java | 17 +++++++++------- api/src/org/labkey/api/util/URLHelper.java | 20 ++++++++++++++----- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/api/src/org/labkey/api/query/QueryParam.java b/api/src/org/labkey/api/query/QueryParam.java index 5bc0f1def03..1547a035ab7 100644 --- a/api/src/org/labkey/api/query/QueryParam.java +++ b/api/src/org/labkey/api/query/QueryParam.java @@ -17,7 +17,6 @@ package org.labkey.api.query; import org.labkey.api.util.SafeToRenderEnum; -import org.labkey.api.util.URLHelper; public enum QueryParam implements SafeToRenderEnum { @@ -32,6 +31,7 @@ public enum QueryParam implements SafeToRenderEnum maxRows, showRows, ignoreFilter, + formDataEncoded, defaultTab, diff --git a/api/src/org/labkey/api/query/QuerySettings.java b/api/src/org/labkey/api/query/QuerySettings.java index e32369b2cf7..be297f0e7bb 100644 --- a/api/src/org/labkey/api/query/QuerySettings.java +++ b/api/src/org/labkey/api/query/QuerySettings.java @@ -75,6 +75,7 @@ public class QuerySettings private boolean _showReports = true; private boolean _ignoreUserFilter; private boolean _ignoreViewFilter; + private boolean _formDataEncoded; private int _maxRows = 100; private boolean _maxRowsSet = false; // Explicitly track setting maxRows, allows for different defaults private long _offset = 0; @@ -183,6 +184,10 @@ public void setSortFilter(PropertyValues pvs) throwParameterParseException(QueryParam.showRows); } } + + String formDataEncoded = _getParameter(param(QueryParam.formDataEncoded)); + if (isNotBlank(formDataEncoded)) + _formDataEncoded = (Boolean) ConvertUtils.convert(formDataEncoded, Boolean.class); } protected String _getParameter(String param) @@ -513,13 +518,11 @@ public URLHelper getReturnUrlHelper(URLHelper defaultURL) public String param(QueryParam param) { - switch (param) + return switch (param) { - case schemaName: - return param.toString(); - default: - return param(param.toString()); - } + case schemaName, formDataEncoded -> param.toString(); + default -> param(param.toString()); + }; } protected String param(String param) @@ -706,7 +709,7 @@ public ActionURL getSortFilterURL() else url = new ActionURL(); url.deleteParameters(); - url.setPropertyValues(_filterSort); + url.setPropertyValues(_filterSort, _formDataEncoded); return url; } diff --git a/api/src/org/labkey/api/util/URLHelper.java b/api/src/org/labkey/api/util/URLHelper.java index 569905fdfcd..55661fd88e4 100644 --- a/api/src/org/labkey/api/util/URLHelper.java +++ b/api/src/org/labkey/api/util/URLHelper.java @@ -676,15 +676,19 @@ public PropertyValues getPropertyValues() return mpvs; } - public void setPropertyValues(PropertyValues pvs) + { + setPropertyValues(pvs, false); + } + + public void setPropertyValues(PropertyValues pvs, boolean isFormDataEncoded) { deleteParameters(); - addPropertyValues(pvs); + addPropertyValues(pvs, isFormDataEncoded); } - public void addPropertyValues(PropertyValues pvs) + public void addPropertyValues(PropertyValues pvs, boolean isFormDataEncoded) { if (null == pvs) return; @@ -693,19 +697,25 @@ public void addPropertyValues(PropertyValues pvs) Object v = pv.getValue(); if (null == v) continue; + + String paramName = pv.getName(); + // Issue 52925 & 52827: App export to csv/tsv ignores filter with column containing double quote + if (isFormDataEncoded) + paramName = paramName.replaceAll("%22", "\"").replaceAll("%2522", "%22"); + if (v.getClass().isArray()) { Object[] a = (Object[])v; for (Object o : a) { if (o != null) - addParameter(pv.getName(), String.valueOf(o)); + addParameter(paramName, String.valueOf(o)); } } else { - addParameter(pv.getName(), String.valueOf(v)); + addParameter(paramName, String.valueOf(v)); } } } From 58818dce464abd911a7b25c0b96cc8ee3c1df93f Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 23 May 2025 10:00:08 -0700 Subject: [PATCH 2/6] Refactor to share util --- api/src/org/labkey/api/util/PageFlowUtil.java | 13 +++++++++++++ api/src/org/labkey/api/util/URLHelper.java | 3 +-- .../labkey/assay/actions/ImportRunApiAction.java | 3 +-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/api/src/org/labkey/api/util/PageFlowUtil.java b/api/src/org/labkey/api/util/PageFlowUtil.java index 8daa412dc9d..a691bb81622 100644 --- a/api/src/org/labkey/api/util/PageFlowUtil.java +++ b/api/src/org/labkey/api/util/PageFlowUtil.java @@ -3129,4 +3129,17 @@ public static HtmlString getDataRegionHtmlForPropertyValues(Map ).appendTo(sb); return HtmlString.unsafe(sb.toString()); } + + /** + * Issue 52925: App export to csv/tsv ignores filter with column containing double quote + * Issue 52119: App issues with assay run properties with special characters + * @param encodedKey The encoded form key by client side `encodeFormDataQuote` util + * @return The decoded raw field name + */ + public static String decodeQuoteEncodedFormDataKey(@Nullable String encodedKey) + { + if (encodedKey == null) + return null; + return encodedKey.replaceAll("%22", "\"").replaceAll("%2522", "%22"); + } } diff --git a/api/src/org/labkey/api/util/URLHelper.java b/api/src/org/labkey/api/util/URLHelper.java index 55661fd88e4..3b9420396b0 100644 --- a/api/src/org/labkey/api/util/URLHelper.java +++ b/api/src/org/labkey/api/util/URLHelper.java @@ -699,9 +699,8 @@ public void addPropertyValues(PropertyValues pvs, boolean isFormDataEncoded) continue; String paramName = pv.getName(); - // Issue 52925 & 52827: App export to csv/tsv ignores filter with column containing double quote if (isFormDataEncoded) - paramName = paramName.replaceAll("%22", "\"").replaceAll("%2522", "%22"); + paramName = PageFlowUtil.decodeQuoteEncodedFormDataKey(paramName); if (v.getClass().isArray()) { diff --git a/assay/src/org/labkey/assay/actions/ImportRunApiAction.java b/assay/src/org/labkey/assay/actions/ImportRunApiAction.java index 086cd69fc8f..efb4b337740 100644 --- a/assay/src/org/labkey/assay/actions/ImportRunApiAction.java +++ b/assay/src/org/labkey/assay/actions/ImportRunApiAction.java @@ -634,8 +634,7 @@ private String parsePropertiesKey(String key) // Issue 52119: account for leading/trailing single quotes and decode double quotes and % if (key.startsWith("'") && key.endsWith("'")) key = key.substring(1, key.length()-1); - key = key.replaceAll("%22", "\""); - key = key.replaceAll("%25", "%"); + key = PageFlowUtil.decodeQuoteEncodedFormDataKey(key); return key; } From 98ebda13c45f6f83feab2832f50b27a0d8408fa5 Mon Sep 17 00:00:00 2001 From: XingY Date: Sun, 25 May 2025 18:23:08 -0700 Subject: [PATCH 3/6] Issue 52984: Add metric for duplicate material names of the same sample type --- experiment/src/org/labkey/experiment/ExperimentModule.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 359e5e14b9e..ee11cb142ce 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -718,7 +718,12 @@ WHERE op.propertyid IN ( results.put("aliquotCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.material where aliquotedfromlsid IS NOT NULL").getObject(Long.class)); results.put("sampleNullAmountCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.material WHERE storedamount IS NULL").getObject(Long.class)); results.put("sampleNegativeAmountCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.material WHERE storedamount < 0").getObject(Long.class)); - results.put("sampleUnitsDifferCount", new SqlSelector(schema, "SELECT COUNT(*) from exp.material m JOIN exp.materialSource s ON m.materialsourceid = s.rowid WHERE m.units != s.metricunit\n").getObject(Long.class)); + results.put("sampleUnitsDifferCount", new SqlSelector(schema, "SELECT COUNT(*) from exp.material m JOIN exp.materialSource s ON m.materialsourceid = s.rowid WHERE m.units != s.metricunit").getObject(Long.class)); + + results.put("duplicateSampleMaterialNameCount", new SqlSelector(schema, "SELECT COUNT(*) as duplicateCount FROM " + + "(SELECT name, cpastype FROM exp.material WHERE cpastype <> 'Material' GROUP BY name, cpastype HAVING COUNT(*) > 1)").getObject(Long.class)); + results.put("duplicateSpecimenMaterialNameCount", new SqlSelector(schema, "SELECT COUNT(*) as duplicateCount FROM " + + "(SELECT name, cpastype FROM exp.material WHERE cpastype = 'Material' GROUP BY name, cpastype HAVING COUNT(*) > 1)").getObject(Long.class)); results.put("dataClassCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.dataclass").getObject(Long.class)); results.put("dataClassRowCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.data WHERE classid IN (SELECT rowid FROM exp.dataclass)").getObject(Long.class)); From 006254cda2c7879b8584614465d4f449067a3f2d Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 26 May 2025 19:04:33 -0700 Subject: [PATCH 4/6] fix sql server --- experiment/src/org/labkey/experiment/ExperimentModule.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index ee11cb142ce..143dbefe190 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -721,9 +721,9 @@ WHERE op.propertyid IN ( results.put("sampleUnitsDifferCount", new SqlSelector(schema, "SELECT COUNT(*) from exp.material m JOIN exp.materialSource s ON m.materialsourceid = s.rowid WHERE m.units != s.metricunit").getObject(Long.class)); results.put("duplicateSampleMaterialNameCount", new SqlSelector(schema, "SELECT COUNT(*) as duplicateCount FROM " + - "(SELECT name, cpastype FROM exp.material WHERE cpastype <> 'Material' GROUP BY name, cpastype HAVING COUNT(*) > 1)").getObject(Long.class)); + "(SELECT name, cpastype FROM exp.material WHERE cpastype <> 'Material' GROUP BY name, cpastype HAVING COUNT(*) > 1) d").getObject(Long.class)); results.put("duplicateSpecimenMaterialNameCount", new SqlSelector(schema, "SELECT COUNT(*) as duplicateCount FROM " + - "(SELECT name, cpastype FROM exp.material WHERE cpastype = 'Material' GROUP BY name, cpastype HAVING COUNT(*) > 1)").getObject(Long.class)); + "(SELECT name, cpastype FROM exp.material WHERE cpastype = 'Material' GROUP BY name, cpastype HAVING COUNT(*) > 1) d").getObject(Long.class)); results.put("dataClassCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.dataclass").getObject(Long.class)); results.put("dataClassRowCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.data WHERE classid IN (SELECT rowid FROM exp.dataclass)").getObject(Long.class)); From 2879a11a15a62af6a080bf990fad6b2c099d48b3 Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 27 May 2025 22:12:12 -0700 Subject: [PATCH 5/6] code review changes --- .../org/labkey/api/action/BaseViewAction.java | 43 ++++++++++++++++++- api/src/org/labkey/api/query/QueryParam.java | 1 - .../org/labkey/api/query/QuerySettings.java | 17 +++----- api/src/org/labkey/api/util/PageFlowUtil.java | 40 +++++++++++------ api/src/org/labkey/api/util/URLHelper.java | 19 +++----- 5 files changed, 82 insertions(+), 38 deletions(-) diff --git a/api/src/org/labkey/api/action/BaseViewAction.java b/api/src/org/labkey/api/action/BaseViewAction.java index 06c1fe3fba4..bf8d225aa31 100644 --- a/api/src/org/labkey/api/action/BaseViewAction.java +++ b/api/src/org/labkey/api/action/BaseViewAction.java @@ -16,6 +16,7 @@ package org.labkey.api.action; +import jakarta.servlet.ServletRequest; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.apache.commons.beanutils.ConversionException; @@ -50,6 +51,7 @@ import org.springframework.beans.TypeMismatchException; import org.springframework.core.MethodParameter; import org.springframework.core.convert.TypeDescriptor; +import org.springframework.lang.Nullable; import org.springframework.validation.BeanPropertyBindingResult; import org.springframework.validation.BindException; import org.springframework.validation.BindingErrorProcessor; @@ -178,12 +180,51 @@ public static PropertyValues getPropertyValuesForFormBinding(PropertyValues pvs, return ret; } + static final String FORM_DATE_ENCODED_PARAM = "formDataEncoded"; + + static public class ViewActionParameterPropertyValues extends ServletRequestParameterPropertyValues + { + + public ViewActionParameterPropertyValues(ServletRequest request) { + this(request, (String)null, (String)null); + } + + public ViewActionParameterPropertyValues(ServletRequest request, @Nullable String prefix, @Nullable String prefixSeparator) + { + super(request, prefix, prefixSeparator); + if (isFormDataEncoded()) + { + for (int i = 0; i < getPropertyValues().length; i++) + { + PropertyValue formDataPropValue = getPropertyValues()[i]; + String propValueName = formDataPropValue.getName(); + String decoded = PageFlowUtil.decodeQuoteEncodedFormDataKey(propValueName); + if (!propValueName.equals(decoded)) + setPropertyValueAt(new PropertyValue(decoded, formDataPropValue.getValue()), i); + } + } + } + + private boolean isFormDataEncoded() + { + PropertyValue formDataPropValue = getPropertyValue(FORM_DATE_ENCODED_PARAM); + if (formDataPropValue != null) + { + Object v = formDataPropValue.getValue(); + String formDataPropValueStr = v == null ? null : String.valueOf(v); + if (StringUtils.isNotBlank(formDataPropValueStr)) + return (Boolean) ConvertUtils.convert(formDataPropValueStr, Boolean.class); + } + + return false; + } + } @Override public ModelAndView handleRequest(@NotNull HttpServletRequest request, @NotNull HttpServletResponse response) throws Exception { if (null == getPropertyValues()) - setProperties(new ServletRequestParameterPropertyValues(request)); + setProperties(new ViewActionParameterPropertyValues(request)); getViewContext().setBindPropertyValues(getPropertyValues()); handleSpecialProperties(); diff --git a/api/src/org/labkey/api/query/QueryParam.java b/api/src/org/labkey/api/query/QueryParam.java index 1547a035ab7..81173c76222 100644 --- a/api/src/org/labkey/api/query/QueryParam.java +++ b/api/src/org/labkey/api/query/QueryParam.java @@ -31,7 +31,6 @@ public enum QueryParam implements SafeToRenderEnum maxRows, showRows, ignoreFilter, - formDataEncoded, defaultTab, diff --git a/api/src/org/labkey/api/query/QuerySettings.java b/api/src/org/labkey/api/query/QuerySettings.java index be297f0e7bb..e32369b2cf7 100644 --- a/api/src/org/labkey/api/query/QuerySettings.java +++ b/api/src/org/labkey/api/query/QuerySettings.java @@ -75,7 +75,6 @@ public class QuerySettings private boolean _showReports = true; private boolean _ignoreUserFilter; private boolean _ignoreViewFilter; - private boolean _formDataEncoded; private int _maxRows = 100; private boolean _maxRowsSet = false; // Explicitly track setting maxRows, allows for different defaults private long _offset = 0; @@ -184,10 +183,6 @@ public void setSortFilter(PropertyValues pvs) throwParameterParseException(QueryParam.showRows); } } - - String formDataEncoded = _getParameter(param(QueryParam.formDataEncoded)); - if (isNotBlank(formDataEncoded)) - _formDataEncoded = (Boolean) ConvertUtils.convert(formDataEncoded, Boolean.class); } protected String _getParameter(String param) @@ -518,11 +513,13 @@ public URLHelper getReturnUrlHelper(URLHelper defaultURL) public String param(QueryParam param) { - return switch (param) + switch (param) { - case schemaName, formDataEncoded -> param.toString(); - default -> param(param.toString()); - }; + case schemaName: + return param.toString(); + default: + return param(param.toString()); + } } protected String param(String param) @@ -709,7 +706,7 @@ public ActionURL getSortFilterURL() else url = new ActionURL(); url.deleteParameters(); - url.setPropertyValues(_filterSort, _formDataEncoded); + url.setPropertyValues(_filterSort); return url; } diff --git a/api/src/org/labkey/api/util/PageFlowUtil.java b/api/src/org/labkey/api/util/PageFlowUtil.java index a691bb81622..960384a869f 100644 --- a/api/src/org/labkey/api/util/PageFlowUtil.java +++ b/api/src/org/labkey/api/util/PageFlowUtil.java @@ -2592,6 +2592,19 @@ public static String joinValuesToString(@NotNull List values, char delim .collect(Collectors.joining(String.valueOf(delimiter))); } + /** + * Issue 52925: App export to csv/tsv ignores filter with column containing double quote + * Issue 52119: App issues with assay run properties with special characters + * @param encodedKey The encoded form key by client side `encodeFormDataQuote` util + * @return The decoded raw field name + */ + public static String decodeQuoteEncodedFormDataKey(@Nullable String encodedKey) + { + if (encodedKey == null) + return null; + return encodedKey.replaceAll("%22", "\"").replaceAll("%2522", "%22"); + } + public static class TestCase extends Assert { @Test @@ -2909,6 +2922,21 @@ public void encodePath() assertEquals("a/b/c", PageFlowUtil.encodePath("a/b/c")); assertEquals("/a/b/c/", PageFlowUtil.encodePath("/a/b/c/")); } + + @Test + public void testDecodeQuoteEncodedFormDataKey() + { + assertEquals("test", decodeQuoteEncodedFormDataKey("test")); + assertEquals("a/b/c", decodeQuoteEncodedFormDataKey("a/b/c")); + assertEquals("a'b.c", decodeQuoteEncodedFormDataKey("a'b.c")); + assertEquals("%", decodeQuoteEncodedFormDataKey("%")); + assertEquals("\"", decodeQuoteEncodedFormDataKey("%22")); + assertEquals("\"\"", decodeQuoteEncodedFormDataKey("%22%22")); + assertEquals("%22", decodeQuoteEncodedFormDataKey("%2522")); + assertEquals("%22%22", decodeQuoteEncodedFormDataKey("%2522%2522")); + assertEquals("%22\"", decodeQuoteEncodedFormDataKey("%2522%22")); + assertEquals("\"22", decodeQuoteEncodedFormDataKey("%2222")); + } } /** @return true if the UrlProvider exists. */ @@ -3130,16 +3158,4 @@ public static HtmlString getDataRegionHtmlForPropertyValues(Map return HtmlString.unsafe(sb.toString()); } - /** - * Issue 52925: App export to csv/tsv ignores filter with column containing double quote - * Issue 52119: App issues with assay run properties with special characters - * @param encodedKey The encoded form key by client side `encodeFormDataQuote` util - * @return The decoded raw field name - */ - public static String decodeQuoteEncodedFormDataKey(@Nullable String encodedKey) - { - if (encodedKey == null) - return null; - return encodedKey.replaceAll("%22", "\"").replaceAll("%2522", "%22"); - } } diff --git a/api/src/org/labkey/api/util/URLHelper.java b/api/src/org/labkey/api/util/URLHelper.java index 3b9420396b0..569905fdfcd 100644 --- a/api/src/org/labkey/api/util/URLHelper.java +++ b/api/src/org/labkey/api/util/URLHelper.java @@ -676,19 +676,15 @@ public PropertyValues getPropertyValues() return mpvs; } - public void setPropertyValues(PropertyValues pvs) - { - setPropertyValues(pvs, false); - } - public void setPropertyValues(PropertyValues pvs, boolean isFormDataEncoded) + public void setPropertyValues(PropertyValues pvs) { deleteParameters(); - addPropertyValues(pvs, isFormDataEncoded); + addPropertyValues(pvs); } - public void addPropertyValues(PropertyValues pvs, boolean isFormDataEncoded) + public void addPropertyValues(PropertyValues pvs) { if (null == pvs) return; @@ -697,24 +693,19 @@ public void addPropertyValues(PropertyValues pvs, boolean isFormDataEncoded) Object v = pv.getValue(); if (null == v) continue; - - String paramName = pv.getName(); - if (isFormDataEncoded) - paramName = PageFlowUtil.decodeQuoteEncodedFormDataKey(paramName); - if (v.getClass().isArray()) { Object[] a = (Object[])v; for (Object o : a) { if (o != null) - addParameter(paramName, String.valueOf(o)); + addParameter(pv.getName(), String.valueOf(o)); } } else { - addParameter(paramName, String.valueOf(v)); + addParameter(pv.getName(), String.valueOf(v)); } } } From 21fc4ee7238f1114d77d08304cd1720c768227b1 Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 27 May 2025 22:23:26 -0700 Subject: [PATCH 6/6] add comment --- api/src/org/labkey/api/action/BaseViewAction.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/api/src/org/labkey/api/action/BaseViewAction.java b/api/src/org/labkey/api/action/BaseViewAction.java index bf8d225aa31..1d9ccdae89b 100644 --- a/api/src/org/labkey/api/action/BaseViewAction.java +++ b/api/src/org/labkey/api/action/BaseViewAction.java @@ -182,6 +182,14 @@ public static PropertyValues getPropertyValuesForFormBinding(PropertyValues pvs, static final String FORM_DATE_ENCODED_PARAM = "formDataEncoded"; + /** + * When a double quote is encountered in a multipart/form-data context, it is encoded as %22 using URL-encoding by browsers. + * This process replaces the double quote with its hexadecimal equivalent in a URL-safe format, preventing it from being misinterpreted as the end of a value or a boundary. + * The consequence of such encoding is we can't distinguish '"' from the actual '%22' in parameter name. + * As a workaround, a client-side util `encodeFormDataQuote` is used to convert %22 to %2522 and " to %22 explicitly, while passing in an additional param formDataEncoded=true. + * This class converts those encoded param names back to its decoded form during PropertyValues binding. + * See Issue 52827, 52925 and 52119 for more information. + */ static public class ViewActionParameterPropertyValues extends ServletRequestParameterPropertyValues {