Handle edge case where attachment part is missing filename #1332
Conversation
|
Hey @jeremy! Wondering if you could take a look at this PR when you get a chance and provide any feedback? |
There was a problem hiding this comment.
Good call. Thanks for digging in.
Noting that the underlying issue is that we rely on presence of filename to imply that a part is an attachment. Perhaps we should change that to be more expansive. Having to provide an "untitled" filename is a workaround that will cause emails to no longer round-trip (resending an email will change a part's filename to "untitled").
| end | ||
| @attachment_name | ||
| end | ||
|
|
There was a problem hiding this comment.
Feels like the message itself should be responsible for determining whether it's an attachment, rather than its Content-Type field.
There was a problem hiding this comment.
Thanks for taking a look and providing feedback @jeremy . I can re-work this a bit and get back to you. Agree that the longer round-trip isn't ideal.
Fixes mikel#1329 First attempt at a fix: mikel#1332 In this second attempt, I tried to scope down the issue to where the issue appears most probable: inline images. We do not add a default filename for the nameless attachment, rather just ensure that the message part is considered an attachment if it is an inline image.
|
Closing in favor of fix in this PR: #1350 |
Fixes mikel#1329 First attempt at a fix: mikel#1332 In this second attempt, I tried to scope down the issue to where the issue appears most probable: inline images. We do not add a default filename for the nameless attachment, rather just ensure that the message part is considered an attachment if it is an inline image.
Fixes #1329
Semi-inspired by php-mime-mail-parser:
Recommending that when the part is missing both content-location and filename, we check the type and provide a default filename if it is an attachment type.