GNU bug report logs - #67002
30.0.50; gnus-article-browse-html-save-cid-content could handle duplicate filenames better

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 67002 in the body.
You can then email your comments to 67002 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Dirk-Jan C. Binnema" <djcb <at> djcbsoftware.nl>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; gnus-article-browse-html-save-cid-content could handle
 duplicate filenames better
Date: Wed, 08 Nov 2023 19:35:24 +0200

[Message part 1 (text/plain, inline)]

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---

Kind regards,
Dirk.

[image_issue_email.txt (text/plain, attachment)]

Message #8 received at 67002 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Dirk-Jan C. Binnema" <djcb <at> djcbsoftware.nl>,
 Eric Abrahamsen <eric <at> ericabrahamsen.net>,
 Andrew G Cohen <cohen <at> andy.bu.edu>
Cc: 67002 <at> debbugs.gnu.org
Subject: Re: bug#67002: 30.0.50;
 gnus-article-browse-html-save-cid-content could handle duplicate
 filenames better
Date: Sat, 18 Nov 2023 11:16:11 +0200

> From: "Dirk-Jan C. Binnema" <djcb <at> djcbsoftware.nl>
> 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?

Message #11 received at 67002 <at> debbugs.gnu.org (full text, mbox):

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67002 <at> debbugs.gnu.org, "Dirk-Jan C. Binnema" <djcb <at> djcbsoftware.nl>,
 Andrew G Cohen <cohen <at> andy.bu.edu>
Subject: Re: bug#67002: 30.0.50; gnus-article-browse-html-save-cid-content
 could handle duplicate filenames better
Date: Sun, 19 Nov 2023 11:58:44 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: "Dirk-Jan C. Binnema" <djcb <at> djcbsoftware.nl>
>> 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.

Thanks,
Eric

Message #14 received at 67002 <at> debbugs.gnu.org (full text, mbox):

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67002 <at> debbugs.gnu.org, "Dirk-Jan C. Binnema" <djcb <at> djcbsoftware.nl>,
 Andrew G Cohen <cohen <at> andy.bu.edu>
Subject: Re: bug#67002: 30.0.50; gnus-article-browse-html-save-cid-content
 could handle duplicate filenames better
Date: Mon, 27 Nov 2023 08:43:59 -0800

Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> From: "Dirk-Jan C. Binnema" <djcb <at> djcbsoftware.nl>
>>> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.