Subject: bug#67002: 30.0.50; gnus-article-browse-html-save-cid-content could handle duplicate filenames better

From: Eric Abrahamsen <[email-masked]>
Date: Mon, 27 Nov 2023 08:43:59 -0800

Eric Abrahamsen <[email-masked]> writes:

> Eli Zaretskii <[email-masked]> writes:
>
>>> From: "Dirk-Jan C. Binnema" <[email-masked]>
>>> Date: Wed, 08 Nov 2023 19:35:24 +0200
>>> 
>>> Dear Gnus,
>>> 
>>> This came up in a mu4e bug:
>>>   https://github.com/djcb/mu/issues/2460
>>> 
>>> (for reference: mu4e is /another/ Emacs MUA, which re-uses a lot of Gnus code)
>>> 
>>> Basically, in the attached html-email there are _two_ CIDs with the same
>>> filename (but with different content-ids)
>>> 
>>> When doing a view-in-browser, gnus-article-browse-html-save-cid-content
>>> uses the filename in favor of the cid, and the first file will be
>>> overwritten by the second.
>>> 
>>> Admittedly, a corner-case. But, it can be fixed.
>>> This fixes it for me:
>>> 
>>> --8<---------------cut here---------------start------------->8---
>>> @@ -2871,11 +2871,14 @@ gnus-article-browse-html-save-cid-content
>>>  			      cid handle directory))
>>>  	      (throw 'found file)))
>>>  	   ((equal (concat "<" cid ">") (mm-handle-id handle))
>>> -	    (setq file (or (mm-handle-filename handle)
>>> -			   (concat
>>> -			    (make-temp-name "cid")
>>> -			    (car (rassoc (car (mm-handle-type handle))
>>> -					 mailcap-mime-extensions))))
>>> +	    ;; Files are randomized since declared filenames may not be unique.
>>> +	    (setq file (format "cid-%d-%s"
>>> +			       (random 99)
>>> +			       (or (mm-handle-filename handle)
>>> +				   (concat
>>> +				    (make-temp-name "cid")
>>> +				    (car (rassoc (car (mm-handle-type handle))
>>> +						 mailcap-mime-extensions)))))
>>>  		  afile (expand-file-name file directory))
>>> --8<---------------cut here---------------end--------------->8---
>>
>> Eric, Andrew: any comments?
>
> We've already got machinery for this sort of thing, in `mm-save-part',
> and `gnus-article-browse-html-save-cid-content' gives me the impression
> that it predates the helper functions in "mm-decode.el". Give me a few
> days, and I'll see if it can't be refactored onto mm-decode.

I got around to looking at this, and I'd misunderstood what was
happening here -- it's kind of a non-interactive context, where the user
is only expecting to view the end result. So I think this is a fine
solution. I'll poke at it a bit more, and close this bug report when the
fix goes in.

Thanks!
Eric