davsclaus commented on code in PR #26661:
URL: https://github.com/apache/camel/pull/26661#discussion_r4060525466
##########
components/camel-mail/src/main/java/org/apache/camel/dataformat/mime/multipart/MimeMultipartDataFormat.java:
##########
@@ -417,7 +418,12 @@ private String getAttachmentKey(BodyPart bp) throws
MessagingException, Unsuppor
if (key == null) {
key = UUID.randomUUID() + "@camel.apache.org";
}
- return MimeUtility.decodeText(key);
+ key = MimeUtility.decodeText(key);
+ // The name is chosen by the sender of the message being unmarshalled,
so normalise it the same
+ // way MailBinding.extractAndNormalizeFileName does before it is used
to identify the attachment:
+ // strip control characters, then reduce it to a leaf name so it
cannot carry path components.
+ key = key.replaceAll("[\n\r\t]", "_");
+ return FileUtil.stripPath(key);
Review Comment:
Non-blocking: this now strips path components from every key, including one
taken from `Content-ID`. A Content-ID local part may legally contain `/` (it is
`atext` in RFC 5322), so `<a/[email protected]>` would become
`[email protected]`. Only the `getFileName()` branch carries sender-chosen path
data, so it may be cleaner to apply `stripPath` there only and leave the
Content-ID / generated keys untouched.
##########
components/camel-mail/src/main/java/org/apache/camel/component/mail/MailBinding.java:
##########
@@ -724,8 +725,12 @@ protected void addAttachmentsToMultipart(
LOG.trace("Attachment #{}: Using content type
resolver: {} resolved content type as: {}", i,
contentTypeResolver, contentType);
if (contentType != null) {
- String value = contentType + "; name=" +
attachmentFilename;
- messageBodyPart.setHeader("Content-Type", value);
+ // The file name comes from the message being
relayed, so it must go out as a
+ // parameter value rather than be concatenated
into the header. ParameterList
+ // quotes and escapes anything that would
otherwise change the header's structure.
+ ContentType parsed = new ContentType(contentType);
Review Comment:
Small behaviour change worth being aware of: `new ContentType(contentType)`
throws `ParseException` if a custom `ContentTypeResolver` returns something
unparsable, where before the raw string was written into the header as-is. I
think failing early with a clear error is the better outcome, just noting it
since it is not mentioned in the description.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
