Issue 52827: File/attachment fields with special characters#6580
Issue 52827: File/attachment fields with special characters#6580
Conversation
…ins tricky characters (unclosed " or containing ::)
# Conflicts: # assay/src/org/labkey/assay/actions/ImportRunApiAction.java
| if (!fieldCommandIndex.equals(commandIndex+"")) | ||
| continue; | ||
| } | ||
| else |
There was a problem hiding this comment.
I think the correct fix is to remove the encoded characters here (early) rather than trying to match them later. Ideally the encoded chars shouldn't "leak" out of code that is pulling values out of the request object.
Then we wouldn't have to handle these characters later.
There was a problem hiding this comment.
The challenge here is that a field might actually contain "%22". Both field" and field%22 will come in as field%22 and we don't know when to decode vs not here.
There was a problem hiding this comment.
This is really an HTTP bug (it's not OK to escape " to %22 without escaping %). So this would break any column with " in its name in a type with a file column then. Not just the file column.
Since these columns could never map to a java property. I'd propose changing BaseColumnInfo.getPropertyName() to replace " with %QUOTE% or something. That should solve the problem by never using the " in the input name. That would fix the dataregion case, anyway. I'm not sure what the editable grid does.
There was a problem hiding this comment.
Editable grid in apps currently doesn't support file fields. The paths supported in Apps are editing detail editing and bulk ediitng. Detail editing uses updateRows api with formData (file, plus json data for other row data) while bulk update uses saveRows. For Apps, only the file field is impacted since other fields are in json. This approach of replace " with %QUOTE% could work for Apps too, but it's also more error prone. Also, it's hard to pick a substitute that absolutely won't collide with real field names, a field might just actually contain %QUOTE%.
labkey-matthewb
left a comment
labkey-matthewb
left a comment
There was a problem hiding this comment.
We can use your approach, but I would add a longer explanation about the problem with the form encoding. We might also consider removing double-quote from generated property name. BaseColumnInfo.getPropertyName().
Co-authored-by: labkey-matthewb <matthewb@labkey.com>
* Try to track down OutOfMemoryError saving custom view
…126, assay unresolved sample lookup issue 52745) (#6609) - AssayResultUpdateService to call syncLineage() on assay result delete rows when sample lookup column exists - ExprColumn null check for various sample lookup fields (IsAliquot, IsPlated, StorageStatus)
# Conflicts: # api/src/org/labkey/api/exp/property/DomainUtil.java
| MultipartHttpServletRequest request = (MultipartHttpServletRequest) getRequest(); | ||
| MultipartFile f = request.getFile(quoteEncodedFieldName); | ||
| isFileColFileRemoved = f != null && (f.getOriginalFilename() == null || f.getOriginalFilename().isEmpty()); | ||
| } | ||
|
|
||
| if (isFileColFileRemoved) | ||
| values.put(column.getName(), null); | ||
| else | ||
| { | ||
| Object value = getTypedValues().get(quoteEncodedFieldName); | ||
| if (value != null) | ||
| values.put(column.getName(), value); | ||
| } |
There was a problem hiding this comment.
this seems like a lot of duplicate code from what is already there below for handling the non-slash column name case (lines 599-607). it seems like the main part is the field name to use in request.getFile(). Could these blocks of code be collapsed and have the conditional part be using either getMultiPartFormFieldName or getFormFieldName? Then after we have the MultipartFile, the rest of the code seems pretty similar.
There was a problem hiding this comment.
This was how it was written in a previous commit, but that broke LKS attachment update. The different part is what value to use for file field: string or File. The only shared code is the following 2 lines so I removed the shared util in the last commit.
MultipartHttpServletRequest request = (MultipartHttpServletRequest) getRequest(); MultipartFile f = request.getFile(quoteEncodedFieldName);
6 participants
Rationale
Related Pull Requests
Changes