From d56b0620cdb88fa6d905551c0ba938f56377a041 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Thu, 19 Mar 2026 15:06:29 -0700 Subject: [PATCH 01/17] PageFlowUtil.encodeFormName() and decodeFormName() --- .../org/labkey/api/action/BaseViewAction.java | 41 ++------- .../org/labkey/api/data/TableViewForm.java | 23 ++--- .../api/dataiterator/DataIteratorUtil.java | 31 ------- .../StandardDataIteratorBuilder.java | 1 - .../labkey/api/exp/property/DomainUtil.java | 2 - api/src/org/labkey/api/jsp/JspBase.java | 5 ++ .../org/labkey/api/query/QueryUpdateForm.java | 17 +--- .../labkey/api/query/excelExportOptions.jsp | 6 +- api/src/org/labkey/api/util/DOM.java | 11 ++- api/src/org/labkey/api/util/InputBuilder.java | 6 +- api/src/org/labkey/api/util/PageFlowUtil.java | 84 ++++++++++++++----- .../org/labkey/api/util/SelectBuilder.java | 2 +- .../org/labkey/api/util/TextAreaBuilder.java | 2 +- .../api/assay/AssayResultUpdateService.java | 3 - .../assay/actions/ImportRunApiAction.java | 4 +- .../core/admin/existingExternalSources.jsp | 4 +- .../labkey/core/admin/existingListValues.jsp | 2 +- .../src/org/labkey/core/login/setPassword.jsp | 5 +- .../experiment/experimentRunQueryHeader.jsp | 2 +- .../query/controllers/QueryController.java | 4 - .../reports/view/reportsWebPartConfig.jsp | 4 +- .../src/org/labkey/query/view/manageApps.jsp | 4 +- .../src/org/labkey/wiki/view/wikiPrintAll.jsp | 2 +- .../src/org/labkey/wiki/view/wikiPrintRaw.jsp | 2 +- 24 files changed, 118 insertions(+), 149 deletions(-) diff --git a/api/src/org/labkey/api/action/BaseViewAction.java b/api/src/org/labkey/api/action/BaseViewAction.java index 0e9e69946d4..c5066898935 100644 --- a/api/src/org/labkey/api/action/BaseViewAction.java +++ b/api/src/org/labkey/api/action/BaseViewAction.java @@ -185,19 +185,11 @@ public static PropertyValues getPropertyValuesForFormBinding(PropertyValues pvs, return ret; } - 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. - */ + /// Some characters can be mishandled by the browser in multipart/formdata requests (e.g. doublequote and backslask). + /// We support an encoding from fields to avoid these characters, see {@link PageFlowUtil#encodeFormName} and {@link PageFlowUtil#decodeFormName}. static public class ViewActionParameterPropertyValues extends ServletRequestParameterPropertyValues { - public ViewActionParameterPropertyValues(ServletRequest request) { this(request, null, null); } @@ -205,31 +197,14 @@ public ViewActionParameterPropertyValues(ServletRequest request) { 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) + for (int i = 0; i < getPropertyValues().length; i++) { - Object v = formDataPropValue.getValue(); - String formDataPropValueStr = v == null ? null : String.valueOf(v); - if (StringUtils.isNotBlank(formDataPropValueStr)) - return (Boolean) ConvertUtils.convert(formDataPropValueStr, Boolean.class); + PropertyValue formDataPropValue = getPropertyValues()[i]; + String propValueName = formDataPropValue.getName(); + String decoded = PageFlowUtil.decodeFormName(propValueName); + if (!propValueName.equals(decoded)) + setPropertyValueAt(new PropertyValue(decoded, formDataPropValue.getValue()), i); } - - return false; } } diff --git a/api/src/org/labkey/api/data/TableViewForm.java b/api/src/org/labkey/api/data/TableViewForm.java index 430646e1cf0..36bdb16ff67 100644 --- a/api/src/org/labkey/api/data/TableViewForm.java +++ b/api/src/org/labkey/api/data/TableViewForm.java @@ -553,22 +553,19 @@ public CaseInsensitiveHashMap getTypedColumns(boolean includeUntyped) for (ColumnInfo column : getTable().getColumns()) { + var fieldName = getFormFieldName(column); + if (hasTypedValue(column)) { values.put(column.getName(), getTypedValue(column)); } - else if (includeUntyped && _stringValues.containsKey(getFormFieldName(column))) + else if (includeUntyped && _stringValues.containsKey(fieldName)) { - values.put(column.getName(), _stringValues.get(getFormFieldName(column))); + values.put(column.getName(), _stringValues.get(fieldName)); } else if (getRequest() instanceof MultipartHttpServletRequest request) { - String fieldName = getMultiPartFormFieldName(column); - Object typedValue = _getTypedValues().get(fieldName); - - if (typedValue != null) - values.put(column.getName(), typedValue); - else if (File.class.equals(column.getJavaClass())) + if (File.class.equals(column.getJavaClass())) { MultipartFile file = request.getFile(fieldName); if (file != null) @@ -587,10 +584,11 @@ else if (File.class.equals(column.getJavaClass())) ColumnInfo mvColumn = getTable().getColumn(column.getMvColumnName()); if (null != mvColumn) { + var mvFieldName = getFormFieldName(mvColumn); if (hasTypedValue(mvColumn)) values.put(mvColumn.getName(), getTypedValue(mvColumn)); - else if (includeUntyped && _stringValues.containsKey(getFormFieldName(mvColumn))) - values.put(mvColumn.getName(), _stringValues.get(getFormFieldName(mvColumn))); + else if (includeUntyped && _stringValues.containsKey(mvFieldName)) + values.put(mvColumn.getName(), _stringValues.get(mvFieldName)); } } } @@ -739,11 +737,6 @@ public String getFormFieldName(@NotNull ColumnInfo column) return column.getName(); } - public String getMultiPartFormFieldName(@NotNull ColumnInfo column) - { - return getFormFieldName(column); - } - @Nullable public ColumnInfo getColumnByFormFieldName(@NotNull String name) { diff --git a/api/src/org/labkey/api/dataiterator/DataIteratorUtil.java b/api/src/org/labkey/api/dataiterator/DataIteratorUtil.java index 813c77e8668..920ff5dda65 100644 --- a/api/src/org/labkey/api/dataiterator/DataIteratorUtil.java +++ b/api/src/org/labkey/api/dataiterator/DataIteratorUtil.java @@ -148,34 +148,6 @@ public enum MatchType alias, jdbcname, tsvColumn, - multiPartFormData() - { - @Override - public String getMatchedName(@Nullable String name) - { - if (name == null) - return null; - // " is encoded as %22 when content-type is "multipart/form-data" (but is not otherwise encoded so decode() does not work) - return name.replaceAll("\"", "%22"); - } - - @Override - public boolean updateRowMap(@NotNull ColumnInfo col, Map rowMap) - { - if (col.getName().contains("\"") && File.class.equals(col.getJavaClass())) - { - // Issue 52827: File/attachment fields with special characters - String quoteEncodedName = DataIteratorUtil.MatchType.multiPartFormData.getMatchedName(col.getName()); - if (rowMap.containsKey(quoteEncodedName)) - { - rowMap.put(col.getName(), rowMap.get(quoteEncodedName)); - rowMap.remove(quoteEncodedName); - return true; - } - } - return false; - } - }, low; public String getMatchedName(@Nullable String name) @@ -209,9 +181,6 @@ protected static Map> _createTableMap(TableInf Map> targetAliasesMap = new CaseInsensitiveHashMap<>(cols.size()*4); - for (ColumnInfo col : cols) - targetAliasesMap.put(MatchType.multiPartFormData.getMatchedName(col.getName()), new Pair<>(col, MatchType.multiPartFormData)); - // should this be under the useImportAliases flag??? for (ColumnInfo col : cols) { diff --git a/api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java b/api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java index ccbd5140882..4bc1a41d7ff 100644 --- a/api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java +++ b/api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java @@ -25,7 +25,6 @@ import org.labkey.api.data.TableInfo; import org.labkey.api.data.validator.ColumnValidator; import org.labkey.api.data.validator.ColumnValidators; -import org.labkey.api.data.validator.RequiredValidator; import org.labkey.api.exp.PropertyDescriptor; import org.labkey.api.exp.PropertyType; import org.labkey.api.exp.api.ExperimentService; diff --git a/api/src/org/labkey/api/exp/property/DomainUtil.java b/api/src/org/labkey/api/exp/property/DomainUtil.java index 303b4445766..29269334501 100644 --- a/api/src/org/labkey/api/exp/property/DomainUtil.java +++ b/api/src/org/labkey/api/exp/property/DomainUtil.java @@ -1543,8 +1543,6 @@ public static ValidationException validateProperties(@Nullable Domain domain, @N else { altNameMap.put(name, name); - altNameMap.put(DataIteratorUtil.MatchType.multiPartFormData.getMatchedName(name), name); - altNameMap.put(name.replaceAll("%22", "\""), name); } } diff --git a/api/src/org/labkey/api/jsp/JspBase.java b/api/src/org/labkey/api/jsp/JspBase.java index 22c7298d790..173e5b8b8b4 100644 --- a/api/src/org/labkey/api/jsp/JspBase.java +++ b/api/src/org/labkey/api/jsp/JspBase.java @@ -211,6 +211,11 @@ public static HtmlString h(URLHelper url) return h(url == null ? null : url.toString()); } + public static HtmlString hname(String name) + { + return HtmlString.of(PageFlowUtil.encodeFormName(name)); + } + // Note: If you have a stream, use LabKeyCollectors.toJsonArray() public static JSONArray toJsonArray(Collection c) { diff --git a/api/src/org/labkey/api/query/QueryUpdateForm.java b/api/src/org/labkey/api/query/QueryUpdateForm.java index c2585120947..e0a982d3b02 100644 --- a/api/src/org/labkey/api/query/QueryUpdateForm.java +++ b/api/src/org/labkey/api/query/QueryUpdateForm.java @@ -107,21 +107,6 @@ else if (c == BACKSLASH) public String getFormFieldName(@NotNull ColumnInfo column) { String columnName = column.getName(); - StringBuilder sb = new StringBuilder(); - for (char c : columnName.toCharArray()) - { - if (SPECIAL_CHARS.indexOf(c) >= 0) - sb.append(BACKSLASH); - sb.append(c); - } - - String fieldName = sb.toString(); - return _ignorePrefix ? fieldName : PREFIX + fieldName; - } - - @Override - public String getMultiPartFormFieldName(@NotNull ColumnInfo column) - { - return DataIteratorUtil.MatchType.multiPartFormData.getMatchedName(getFormFieldName(column)); + return _ignorePrefix ? columnName : PREFIX + columnName; } } diff --git a/api/src/org/labkey/api/query/excelExportOptions.jsp b/api/src/org/labkey/api/query/excelExportOptions.jsp index a313c054d9c..a27644bff4a 100644 --- a/api/src/org/labkey/api/query/excelExportOptions.jsp +++ b/api/src/org/labkey/api/query/excelExportOptions.jsp @@ -64,16 +64,16 @@ %> - + - + <% if (model.getIqyURL() != null) { %> - + <% } %> diff --git a/api/src/org/labkey/api/util/DOM.java b/api/src/org/labkey/api/util/DOM.java index 1feb02da7b1..f31179179b2 100644 --- a/api/src/org/labkey/api/util/DOM.java +++ b/api/src/org/labkey/api/util/DOM.java @@ -422,7 +422,16 @@ Appendable render(Appendable builder, Object value) throws IOException min, multiple, muted, - name, + name + { + @Override + Appendable render(Appendable builder, Object value) throws IOException + { + if (value instanceof String s) + value = PageFlowUtil.encodeFormName(s); + return super.render(builder, value); + } + }, nonce, novalidate { diff --git a/api/src/org/labkey/api/util/InputBuilder.java b/api/src/org/labkey/api/util/InputBuilder.java index d28fb33ce83..3648f245284 100644 --- a/api/src/org/labkey/api/util/InputBuilder.java +++ b/api/src/org/labkey/api/util/InputBuilder.java @@ -898,6 +898,10 @@ protected final String h(int i) { return String.valueOf(i); } + protected final String hname(String name) + { + return PageFlowUtil.filter(PageFlowUtil.encodeFormName(name)); + } protected void doInput(Appendable sb) throws IOException { @@ -911,7 +915,7 @@ protected void doInput(Appendable sb) throws IOException sb.append("\""); } - sb.append(" name=\"").append(h(getName())).append("\""); + sb.append(" name=\"").append(hname(getName())).append("\""); var id = generateId("input"); diff --git a/api/src/org/labkey/api/util/PageFlowUtil.java b/api/src/org/labkey/api/util/PageFlowUtil.java index a39139f9d5f..af6db280901 100644 --- a/api/src/org/labkey/api/util/PageFlowUtil.java +++ b/api/src/org/labkey/api/util/PageFlowUtil.java @@ -2677,17 +2677,30 @@ private static boolean shouldEscapeForExport(@NotNull String value) } - /** - * 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) + static final String FIELD_ENCODED_PREFIX = "%_"; + + /// Because of various bugs related encoding of INPUT.name values in multipart/formdata, we now recommend encoding + /// all names in all forms. + /// The choice of using encodeURI component is somewhat arbitrary, any encoding that can remove + /// double-quote and backslash from the name would be fine. + public static String encodeFormName(String name) + { + final String escapeChar = "%"; + final String problemChars = "\\\""; + final String unclean = escapeChar + problemChars; + if (!StringUtils.containsAny(name, unclean)) + return name; + var ret = FIELD_ENCODED_PREFIX + encodeURIComponent(name); + assert !StringUtils.containsAny(ret, problemChars); + return ret; + } + + public static String decodeFormName(String name) { - if (encodedKey == null) - return null; - return encodedKey.replaceAll("%22", "\"").replaceAll("%2522", "%22"); + if (!name.startsWith(FIELD_ENCODED_PREFIX)) + return name; + var ret = decode(name.substring(2)); + return ret; } public static class TestCase extends Assert @@ -3095,19 +3108,46 @@ public void encodePath() assertEquals("/a/b/c/", PageFlowUtil.encodePath("/a/b/c/")); } + private void assertEncodeDecode(String test) + { + assertEquals(test, decodeFormName(encodeFormName(test))); + } + + private void assertReencode(String a) + { + // We want to make sure there are no ambiguous encodings + var b = encodeFormName(a); + var c = encodeFormName(b); + if (a.equals(b)) + assertEquals(a,c); + else + assertNotEquals(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")); + public void testFormNameEncoding() + { + assertEncodeDecode("test"); + assertEncodeDecode("a/b/c"); + assertEncodeDecode("a'b.c"); + assertEncodeDecode("%"); + assertEncodeDecode("\""); + assertEncodeDecode("\"\""); + assertEncodeDecode("%22"); + assertEncodeDecode("%22%22"); + assertEncodeDecode("%22\""); + assertEncodeDecode("\"22"); + + assertReencode("test"); + assertReencode("a/b/c"); + assertReencode("a'b.c"); + assertReencode("%"); + assertReencode("\""); + assertReencode("\"\""); + assertReencode("%22"); + assertReencode("%22%22"); + assertReencode("%22\""); + assertReencode("\"22"); } } diff --git a/api/src/org/labkey/api/util/SelectBuilder.java b/api/src/org/labkey/api/util/SelectBuilder.java index 7145c4f0a51..521814cccd3 100644 --- a/api/src/org/labkey/api/util/SelectBuilder.java +++ b/api/src/org/labkey/api/util/SelectBuilder.java @@ -167,7 +167,7 @@ protected void doInput(Appendable sb) throws IOException { var id = StringUtils.defaultIfBlank(getId(), generateId("select")); - sb.append("> resolveRows( ColumnInfo col = columnInfoMap.get(entry.getKey()); if (col != null && !row.containsKey(entry.getKey())) { - if (DataIteratorUtil.MatchType.multiPartFormData.updateRowMap(col, row)) - continue; - // use column names for existing row values row.put(col.getName(), entry.getValue()); } diff --git a/assay/src/org/labkey/assay/actions/ImportRunApiAction.java b/assay/src/org/labkey/assay/actions/ImportRunApiAction.java index 54d26bc3220..36302c7d5f3 100644 --- a/assay/src/org/labkey/assay/actions/ImportRunApiAction.java +++ b/assay/src/org/labkey/assay/actions/ImportRunApiAction.java @@ -746,9 +746,9 @@ private static String parsePropertiesKey(String key) return null; // Issue 52119: account for leading/trailing single quotes and decode double quotes and % - if (key.startsWith("'") && key.endsWith("'")) + if (key.length() >= 2 && key.startsWith("'") && key.endsWith("'")) key = key.substring(1, key.length()-1); - key = PageFlowUtil.decodeQuoteEncodedFormDataKey(key); + key = PageFlowUtil.decodeFormName(key); return key; } diff --git a/core/src/org/labkey/core/admin/existingExternalSources.jsp b/core/src/org/labkey/core/admin/existingExternalSources.jsp index 4a8ba828a67..0326dc64a57 100644 --- a/core/src/org/labkey/core/admin/existingExternalSources.jsp +++ b/core/src/org/labkey/core/admin/existingExternalSources.jsp @@ -92,8 +92,8 @@ String hostId = "host" + num; %> - - + + - + <% diff --git a/core/src/org/labkey/core/login/setPassword.jsp b/core/src/org/labkey/core/login/setPassword.jsp index 9cb53764bb8..33da7c36ba1 100644 --- a/core/src/org/labkey/core/login/setPassword.jsp +++ b/core/src/org/labkey/core/login/setPassword.jsp @@ -76,7 +76,7 @@ @@ -95,7 +95,7 @@ @@ -103,7 +103,6 @@ if (rule.shouldShowPasswordGuidance() && firstPassword) { firstPasswordId = input.getObject().toString(); - %> Your browser does not support the HTML5 canvas element. diff --git a/experiment/src/org/labkey/experiment/experimentRunQueryHeader.jsp b/experiment/src/org/labkey/experiment/experimentRunQueryHeader.jsp index a58220ce4e3..eccfa7725b9 100644 --- a/experiment/src/org/labkey/experiment/experimentRunQueryHeader.jsp +++ b/experiment/src/org/labkey/experiment/experimentRunQueryHeader.jsp @@ -33,7 +33,7 @@ { if (!"experimentRunFilter".equals(params.getKey())) { %> - + <% } } %>

diff --git a/query/src/org/labkey/query/controllers/QueryController.java b/query/src/org/labkey/query/controllers/QueryController.java index 874f0a0ad70..1834920b79d 100644 --- a/query/src/org/labkey/query/controllers/QueryController.java +++ b/query/src/org/labkey/query/controllers/QueryController.java @@ -141,7 +141,6 @@ import org.labkey.api.data.dialect.SqlDialect; import org.labkey.api.dataiterator.DataIteratorBuilder; import org.labkey.api.dataiterator.DataIteratorContext; -import org.labkey.api.dataiterator.DataIteratorUtil; import org.labkey.api.dataiterator.DetailedAuditLogDataIterator; import org.labkey.api.dataiterator.ListofMapsDataIterator; import org.labkey.api.exceptions.OptimisticConflictException; @@ -4861,9 +4860,6 @@ private void addRowAttachments(TableInfo tableInfo, Map rowMap, rowMap.put(fieldKey, file.isEmpty() ? null : file); } } - - for (ColumnInfo col : tableInfo.getColumns()) - DataIteratorUtil.MatchType.multiPartFormData.updateRowMap(col, rowMap); } protected boolean isSuccessOnValidationError() diff --git a/query/src/org/labkey/query/reports/view/reportsWebPartConfig.jsp b/query/src/org/labkey/query/reports/view/reportsWebPartConfig.jsp index eeca9853197..5e46ee09588 100644 --- a/query/src/org/labkey/query/reports/view/reportsWebPartConfig.jsp +++ b/query/src/org/labkey/query/reports/view/reportsWebPartConfig.jsp @@ -106,7 +106,7 @@

<% addHandler("showTabs", "click", "return onShowTabs(this.checked);"); %> - +
Maximum 1,048,576 rows and 16,384 columns.
Maximum 65,536 rows and 256 columns.
/>/> <%=isTroubleshooter ? HtmlString.EMPTY_STRING : diff --git a/core/src/org/labkey/core/admin/existingListValues.jsp b/core/src/org/labkey/core/admin/existingListValues.jsp index fe49ec75110..ccc57304a47 100644 --- a/core/src/org/labkey/core/admin/existingListValues.jsp +++ b/core/src/org/labkey/core/admin/existingListValues.jsp @@ -83,7 +83,7 @@ String inputNameExisting = "existingValue" + num; %>
/>/> <%=isTroubleshooter ? HtmlString.EMPTY_STRING : button("Delete").primary(true).onClick("return deleteExisting(\"" + h(value) + "\");") %>
Show Tabs:<%=helpPopup("Show tabs", "Some reports/charts may be rendered with multiple tabs showing. Select this option to only show the primary one.")%>>>
Visible Report Sections:<%=helpPopup("Show Report sections", @@ -120,7 +120,7 @@
- +