Modify getMessagePart to only return selected message part if it
isn't an attachment, to prevent accidentally quoting a (possibly)
binary attachment file.

Signed-off-by: Sebastien Binet <s@sbinet.org>
---
 commands/msg/utils.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/commands/msg/utils.go b/commands/msg/utils.go
index d6dffd50..022e79bf 100644
--- a/commands/msg/utils.go
+++ b/commands/msg/utils.go
@@ -63,7 +63,8 @@ func (h *helper) messages() ([]*models.MessageInfo, error) {
 
 func getMessagePart(msg *models.MessageInfo, provider app.ProvidesMessage) []int {
 	p := provider.SelectedMessagePart()
-	if p != nil {
+	// return selected part only if not an attachment.
+	if p != nil && (p.Part == nil || p.Part.Disposition != "attachment") {
 		return p.Index
 	}
 	for _, mime := range config.Viewer.Alternatives {
-- 
2.43.1

aerc/patches: SUCCESS in 2m17s

[commands/msg: only select non-attachment part for reply/forward][0] from [Sebastien Binet][1]

[0]: https://lists.sr.ht/~rjarry/aerc-devel/patches/49490
[1]: s@sbinet.org

✓ #1149893 SUCCESS aerc/patches/alpine-edge.yml https://builds.sr.ht/~rjarry/job/1149893
✓ #1149894 SUCCESS aerc/patches/openbsd.yml     https://builds.sr.ht/~rjarry/job/1149894

On Wed Feb 14, 2024 at 15:02, Sebastien Binet <s@sbinet.org> wrote:
> Modify getMessagePart to only return selected message part if it
> isn't an attachment, to prevent accidentally quoting a (possibly)
> binary attachment file.
>

Can't we detect instead if it is a binary file? I actually see the merit in
replying to say an attached README.md. Although in that case you might need
parts of text/html as well ... For that you probably need to pipe the message
part to your clipboard and insert in the composer. 

> @@ -63,7 +63,8 @@ func (h *helper) messages() ([]*models.MessageInfo, error) {
>  
>  func getMessagePart(msg *models.MessageInfo, provider app.ProvidesMessage) []int {
>  	p := provider.SelectedMessagePart()
> -	if p != nil {
> +	// return selected part only if not an attachment.
> +	if p != nil && (p.Part == nil || p.Part.Disposition != "attachment") {
>  		return p.Index
>  	}

if we do go down this route, we should also exclude inline





-- 
+36305425054
bence.ferdinandy.com

On Wed Feb 14, 2024 at 16:24 CET, Bence Ferdinandy wrote:
>
> On Wed Feb 14, 2024 at 15:02, Sebastien Binet <s@sbinet.org> wrote:
> > Modify getMessagePart to only return selected message part if it
> > isn't an attachment, to prevent accidentally quoting a (possibly)
> > binary attachment file.
> >
>
> Can't we detect instead if it is a binary file? I actually see the merit in
> replying to say an attached README.md. Although in that case you might need
> parts of text/html as well ... For that you probably need to pipe the message
> part to your clipboard and insert in the composer.

is there any agreed upon criteria for deciding a file is binary ?
mime type ? which one(s) ? should this list be configurable ?

perhaps we should leave it to the user to do the right thing (and put this
patch to the trash.)

-s

On Wed 14 Feb 2024 18:41:18, Sebastien Binet wrote:
> is there any agreed upon criteria for deciding a file is binary ?
> mime type ? which one(s) ? should this list be configurable ?

At least git does it by checking the first 1000 byte of the file and
checking if there is a \0 if memory serves it well.

-- 
Moritz Poldrack
https://moritz.sh

> Use of software is governed by the terms of the end user license
> agreement.

While I agree this could be improved along Bence's proposals, I also think
that this patch is already better than what we had.

I'm having trouble in the following situation, however:

(multipart/related)
(multipart/alternative)
      (text/plain)
      (text/html)
    image003.jpg (image/jpeg)
  attachment.pdf (application/pdf)     

If I reply while having "attachment.pdf", it works allright and I get to
reply to the text/plain part. However, if I reply while having
"image003.jpg" selected, I get to reply to the binary representation of
that file.

:-?

Sebastien Binet, Feb 14, 2024 at 18:41:
> is there any agreed upon criteria for deciding a file is binary ?
> mime type ? which one(s) ? should this list be configurable ?
>
> perhaps we should leave it to the user to do the right thing (and put this
> patch to the trash.)

I am assuming that this patch NEEDS_REVISION. Let me know if I am 
mistaken.

On 10/03/2024, 17:23, Robin Jarry wrote:
> Sebastien Binet, Feb 14, 2024 at 18:41:
> > is there any agreed upon criteria for deciding a file is binary ?
> > mime type ? which one(s) ? should this list be configurable ?
> >
> > perhaps we should leave it to the user to do the right thing (and put this
> > patch to the trash.)
>
> I am assuming that this patch NEEDS_REVISION. Let me know if I am 
> mistaken.

Maybe another, better solution for this would be to make a default to
reply to the text/plain part, and then add an option to reply to the
selected part (or any other, fwiw) if the user needs it?

On Sun Mar 10, 2024 at 19:39, inwit <inwit@sindominio.net> wrote:
> On 10/03/2024, 17:23, Robin Jarry wrote:
> > Sebastien Binet, Feb 14, 2024 at 18:41:
> > > is there any agreed upon criteria for deciding a file is binary ?
> > > mime type ? which one(s) ? should this list be configurable ?
> > >
> > > perhaps we should leave it to the user to do the right thing (and put this
> > > patch to the trash.)
> >
> > I am assuming that this patch NEEDS_REVISION. Let me know if I am 
> > mistaken.
>
> Maybe another, better solution for this would be to make a default to
> reply to the text/plain part, and then add an option to reply to the
> selected part (or any other, fwiw) if the user needs it?

That actually sounds like a pretty sane solution.