Handle edge case when inline image attachment part is missing a filename #1350
+118
−8
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
lao9
force-pushed
the
lo/message-handle-attachment
branch
from
79c3c8d to
401b4e5
Compare
|
I'd love to see this merged in soon to fix an issue we’re seeing. @jeremy, anything you're waiting on before accepting this patch? |
jeremy
reviewed
Nov 15, 2019
| @@ -2135,6 +2135,14 @@ def find_attachment | |||
| filename | |||
| end | |||
|
|
|||
| def inline_image? | |||
| image? && inline? | |||
| end | |||
There was a problem hiding this comment.
inline? is a method on the Mail::Part subclass, so this'll break for a Mail::Message itself.
Could move this into Mail::Part and override the attachment? method, perhaps?
# class Part < Message
def attachment?
super || inline_image?
end
# …
def inline_image?
# …There was a problem hiding this comment.
Thanks for the feedback - I'll try to get this in next by next week!
|
(Moved to 2.8.0 release as this technically breaks compatibility with current stable 2.7.x.) |
|
@lao9 Please update this to latest master. Moved to 2.9 as it breaks compatibility with 2.7 and 2.8. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1329
First attempt at a fix: #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.