[GH-ISSUE #249] 🕷 [Major] Reply & Reply All completly broken #213

Closed
opened 2026-02-25 21:34:27 +03:00 by kerem · 22 comments
Owner

Originally created by @ulfgebhardt on GitHub (Jan 5, 2018).
Original GitHub issue: https://github.com/cypht-org/cypht/issues/249

Originally assigned to: @jasonmunro on GitHub.

The Reply & Reply all Functionality is complete broken

Subject: bla
Date: Fri, 05 Jan 2018 07:36:16 +0100
From: Peter World <peter.world@world-online.de>
To: "" <peter.world@world-online.de>

Reply and Reply All results in empty To-Field in Compose


Subject: bla4
Date: Fri, 05 Jan 2018 07:55:59 +0100
From: Peter World <peter.world@world-online.de>
To: Peter World <peter.world@world-online.de>
Cc: "" <petra@web.de>

Reply results in:

To: SRS0=Uk67=EA=world-online.de=peter.world@srs.web.de

Reply All results in:

To: SRS0=Uk67=EA=world-online.de=peter.world@srs.web.de
Cc: petra@web.de

Rev: ed4cd35

Grüße Ulf

<3

Originally created by @ulfgebhardt on GitHub (Jan 5, 2018). Original GitHub issue: https://github.com/cypht-org/cypht/issues/249 Originally assigned to: @jasonmunro on GitHub. The Reply & Reply all Functionality is complete broken ``` Subject: bla Date: Fri, 05 Jan 2018 07:36:16 +0100 From: Peter World <peter.world@world-online.de> To: "" <peter.world@world-online.de> ``` Reply and Reply All results in empty To-Field in Compose --- ``` Subject: bla4 Date: Fri, 05 Jan 2018 07:55:59 +0100 From: Peter World <peter.world@world-online.de> To: Peter World <peter.world@world-online.de> Cc: "" <petra@web.de> ``` Reply results in: > To: SRS0=Uk67=EA=world-online.de=peter.world@srs.web.de Reply All results in: > To: SRS0=Uk67=EA=world-online.de=peter.world@srs.web.de > Cc: petra@web.de Rev: ed4cd35 Grüße Ulf <3
kerem 2026-02-25 21:34:27 +03:00
  • closed this issue
  • added the
    smtp
    label
Author
Owner

@jasonmunro commented on GitHub (Jan 5, 2018):

I'm not seeing this behavior. What is the reply-to header in this case? That is what we use as a primary reference for a reply, otherwise we use the from field.

<!-- gh-comment-id:355593016 --> @jasonmunro commented on GitHub (Jan 5, 2018): I'm not seeing this behavior. What is the reply-to header in this case? That is what we use as a primary reference for a reply, otherwise we use the from field.
Author
Owner

@ulfgebhardt commented on GitHub (Jan 5, 2018):

Sent you two EMails, containing all Headers, Original Message and a Screenshot.

a. I think it might be the National Character Guy who is the reason for this.
b. Maybe the dot in my Address causes the Problem?

Grüße Ulf

<3

<!-- gh-comment-id:355604980 --> @ulfgebhardt commented on GitHub (Jan 5, 2018): Sent you two EMails, containing all Headers, Original Message and a Screenshot. a. I think it might be the National Character Guy who is the reason for this. b. Maybe the dot in my Address causes the Problem? Grüße Ulf <3
Author
Owner

@jasonmunro commented on GitHub (Jan 5, 2018):

For some reason we are replying to the return-path field (which is allowed, but we should only be using that if we don't find valid addresses in reply-to or from, which your messages have). Your return-path header is set to:

SRS0=Uk67=EA=world-online.de=peter.world@srs.web.de

Which explains why you are seeing that in the compose form on reply. Not yet sure why the reply-to and from fields are not being picked up.

<!-- gh-comment-id:355606112 --> @jasonmunro commented on GitHub (Jan 5, 2018): For some reason we are replying to the return-path field (which is allowed, but we should only be using that if we don't find valid addresses in reply-to or from, which your messages have). Your return-path header is set to: SRS0=Uk67=EA=world-online.de=peter.world@srs.web.de Which explains why you are seeing that in the compose form on reply. Not yet sure why the reply-to and from fields are not being picked up.
Author
Owner

@ulfgebhardt commented on GitHub (Jan 5, 2018):

I think this is because i set my old EMail Provider to forward all EMails to my new Address.

The Provider is www.web.de - Its a pretty big one in Germany, they should stick to the Standards i would assume.

Im unsure if the ReturnPath is valid, but it looks like a converted URL

"üäö.de" -> xn--4ca0bq.de

Thats some standarized retarded mechanism to translate URLs - should be called IDNA-Domain

http://tools.macx.de/tools/idnconvert.html

Can you please censor the Return-Path in your Message above? one can read my EMail with some fantasy or a proper converter.

Grüße Ulf

<3

<!-- gh-comment-id:355608541 --> @ulfgebhardt commented on GitHub (Jan 5, 2018): I think this is because i set my old EMail Provider to forward all EMails to my new Address. The Provider is www.web.de - Its a pretty big one in Germany, they should stick to the Standards i would assume. Im unsure if the ReturnPath is valid, but it looks like a converted URL > "üäö.de" -> xn--4ca0bq.de Thats some standarized retarded mechanism to translate URLs - should be called IDNA-Domain > http://tools.macx.de/tools/idnconvert.html Can you please censor the Return-Path in your Message above? one can read my EMail with some fantasy or a proper converter. Grüße Ulf <3
Author
Owner

@jasonmunro commented on GitHub (Jan 5, 2018):

Can you please censor the Return-Path in your Message above? one can read my EMail with some fantasy or a proper converter.

Done, sorry about that.

<!-- gh-comment-id:355609277 --> @jasonmunro commented on GitHub (Jan 5, 2018): > Can you please censor the Return-Path in your Message above? one can read my EMail with some fantasy or a proper converter. Done, sorry about that.
Author
Owner

@jasonmunro commented on GitHub (Jan 5, 2018):

putting aside the fact that the return-path is unusual, I can't see why the code is actually using it. I tested the relevant function:

https://github.com/jasonmunro/cypht/blob/master/modules/core/message_functions.php#L106

with the reply-to header from the test mail you sent me, and it found it and used it. The only way it should pick up the return-path is if the reply-to, from, and sender don't have a valid address. Weird.

<!-- gh-comment-id:355610177 --> @jasonmunro commented on GitHub (Jan 5, 2018): putting aside the fact that the return-path is unusual, I can't see why the code is actually using it. I tested the relevant function: https://github.com/jasonmunro/cypht/blob/master/modules/core/message_functions.php#L106 with the reply-to header from the test mail you sent me, and it found it and used it. The only way it should pick up the return-path is if the reply-to, from, and sender don't have a valid address. Weird.
Author
Owner

@ulfgebhardt commented on GitHub (Jan 5, 2018):

If it helps i can provide Video or more Info, like config etc.

<!-- gh-comment-id:355612493 --> @ulfgebhardt commented on GitHub (Jan 5, 2018): If it helps i can provide Video or more Info, like config etc.
Author
Owner

@jasonmunro commented on GitHub (Jan 5, 2018):

I just took the test and test2 mails you sent me and manually created entries in a local maildir. Reply and reply-all for the first test are working fine for me. Reply and reply-all for the second test end up with a blank compose form. Digging into that now. In either case I'm not seeing the return-path being used.

<!-- gh-comment-id:355613768 --> @jasonmunro commented on GitHub (Jan 5, 2018): I just took the test and test2 mails you sent me and manually created entries in a local maildir. Reply and reply-all for the first test are working fine for me. Reply and reply-all for the second test end up with a blank compose form. Digging into that now. In either case I'm not seeing the return-path being used.
Author
Owner

@jasonmunro commented on GitHub (Jan 5, 2018):

Just to confirm, you are sure you don't have any local changes and that everything is updated to the latest git master branch?

<!-- gh-comment-id:355613902 --> @jasonmunro commented on GitHub (Jan 5, 2018): Just to confirm, you are sure you don't have any local changes and that everything is updated to the latest git master branch?
Author
Owner

@jasonmunro commented on GitHub (Jan 5, 2018):

I think I'm losing my mind. As soon as I started debugging this, it started working fine. Now I get correct results for reply and reply-all for both messages.

<!-- gh-comment-id:355614566 --> @jasonmunro commented on GitHub (Jan 5, 2018): I think I'm losing my mind. As soon as I started debugging this, it started working fine. Now I get correct results for reply and reply-all for both messages.
Author
Owner

@ulfgebhardt commented on GitHub (Jan 5, 2018):

image

image

I compared the hm3.ini with hm3.sample.ini - nothing important there

console

This seems 2 be ok aswell

How i generated the Branch: pull yours, branch my master, rebase my setting branch in it

Thanks for the great Work, quick answers and awesome support! Don't get over your Head, even tho this issue is marked as major, i can work around it.

I can tell that this Problem did not exist before (to my notice) till i updated (i think from 33bee91).

Grüße Ulf

<3

<!-- gh-comment-id:355618311 --> @ulfgebhardt commented on GitHub (Jan 5, 2018): ![image](https://user-images.githubusercontent.com/1238238/34620721-bbdb1e50-f246-11e7-87b6-c4ddbded1db9.png) ![image](https://user-images.githubusercontent.com/1238238/34620755-e0a7df66-f246-11e7-81cc-0fc098ab0e41.png) I compared the hm3.ini with hm3.sample.ini - nothing important there ![console](https://user-images.githubusercontent.com/1238238/34621075-017c905a-f248-11e7-8801-011c607d08d9.PNG) This seems 2 be ok aswell How i generated the Branch: pull yours, branch my master, rebase my setting branch in it Thanks for the great Work, quick answers and awesome support! Don't get over your Head, even tho this issue is marked as major, i can work around it. I can tell that this Problem did not exist before (to my notice) till i updated (i think from 33bee91). Grüße Ulf <3
Author
Owner

@dumblob commented on GitHub (Jan 6, 2018):

I'm not an expert on PHP, but I asked myself how would the code behave if we changed

function format_reply_fields($body, $headers, $struct, $html, $output_mod, $type='reply', $excluded) {
    $msg_to = '';
    $msg = '';
    $subject = reply_to_subject($headers, $type);
    $msg_id = reply_to_id($headers, $type);
    list($msg_to, $msg_cc) = reply_to_address($headers, $type, $excluded);
    $lead_in = reply_lead_in($headers, $type, $msg_to, $output_mod);
    $msg = reply_format_body($headers, $body, $lead_in, $type, $struct, $html);
    return array($msg_to, $msg_cc, $subject, $msg, $msg_id);
}

to

function format_reply_fields($body, $headers, $struct, $html, $output_mod, $type='reply', $excluded) {
    $msg = '';
    $subject = reply_to_subject($headers, $type);
    $msg_id = reply_to_id($headers, $type);
    list($msg_to, $msg_cc) = reply_to_address($headers, $type, $excluded);
    $lead_in = reply_lead_in($headers, $type, $msg_to, $output_mod);
    $msg = reply_format_body($headers, $body, $lead_in, $type, $struct, $html);
    return array($msg_to, $msg_cc, $subject, $msg, $msg_id);
}

(note the missing $msg_to = '';)

There might be a difference in initialization of declared and undeclared variables using list($x, $y) = return_list_of_x_and_y(). There also seems to be a significant difference between PHP versions - see http://php.net/manual/en/function.list.php .

<!-- gh-comment-id:355742459 --> @dumblob commented on GitHub (Jan 6, 2018): I'm not an expert on PHP, but I asked myself how would the code behave if we changed ~~~~php function format_reply_fields($body, $headers, $struct, $html, $output_mod, $type='reply', $excluded) { $msg_to = ''; $msg = ''; $subject = reply_to_subject($headers, $type); $msg_id = reply_to_id($headers, $type); list($msg_to, $msg_cc) = reply_to_address($headers, $type, $excluded); $lead_in = reply_lead_in($headers, $type, $msg_to, $output_mod); $msg = reply_format_body($headers, $body, $lead_in, $type, $struct, $html); return array($msg_to, $msg_cc, $subject, $msg, $msg_id); } ~~~~ to ~~~~php function format_reply_fields($body, $headers, $struct, $html, $output_mod, $type='reply', $excluded) { $msg = ''; $subject = reply_to_subject($headers, $type); $msg_id = reply_to_id($headers, $type); list($msg_to, $msg_cc) = reply_to_address($headers, $type, $excluded); $lead_in = reply_lead_in($headers, $type, $msg_to, $output_mod); $msg = reply_format_body($headers, $body, $lead_in, $type, $struct, $html); return array($msg_to, $msg_cc, $subject, $msg, $msg_id); } ~~~~ (note the missing `$msg_to = '';`) There might be a difference in initialization of declared and undeclared variables using `list($x, $y) = return_list_of_x_and_y()`. There also seems to be a significant difference between PHP versions - see http://php.net/manual/en/function.list.php .
Author
Owner

@jasonmunro commented on GitHub (Jan 11, 2018):

@dumblob Thanks for the idea. It's my understanding we can remove both the $msg_to = ''; and $msg = ''; lines, since they are always being set to something later on in the function. I read over the subtle differences in list() between PHP versions, and I don't think that is related to this either, however it would be useful to see if I can duplicate the problem with the same PHP version as @ulfgebhardt

@ulfgebhardt What PHP version are you using?

<!-- gh-comment-id:356809785 --> @jasonmunro commented on GitHub (Jan 11, 2018): @dumblob Thanks for the idea. It's my understanding we can remove both the $msg_to = ''; and $msg = ''; lines, since they are always being set to something later on in the function. I read over the subtle differences in list() between PHP versions, and I don't think that is related to this either, however it would be useful to see if I can duplicate the problem with the same PHP version as @ulfgebhardt @ulfgebhardt What PHP version are you using?
Author
Owner

@ulfgebhardt commented on GitHub (Jan 11, 2018):

Hey Jason,

i am using Running latest 5.6.33+dfsg-0+deb8u1

so i am not using php 7, but latest php 5.

Grüße Ulf

<3

<!-- gh-comment-id:356864028 --> @ulfgebhardt commented on GitHub (Jan 11, 2018): Hey Jason, i am using ```Running latest 5.6.33+dfsg-0+deb8u1``` so i am not using php 7, but latest php 5. Grüße Ulf <3
Author
Owner

@jasonmunro commented on GitHub (Jan 16, 2018):

Just tested with PHP 5.6.26-1 on Debian and was able to reply + reply all to both test messages without issue, so I still can't reproduce or explain this problem.

<!-- gh-comment-id:358036064 --> @jasonmunro commented on GitHub (Jan 16, 2018): Just tested with PHP 5.6.26-1 on Debian and was able to reply + reply all to both test messages without issue, so I still can't reproduce or explain this problem.
Author
Owner

@ulfgebhardt commented on GitHub (Jan 16, 2018):

Hey Jason,

I have this issue particular when i try to reply to my own EMails.
Maybe its address-book related. For other mails it works fine.

I'm quite busy at the moment; as soon as i get the time i will update cypht again and make some checks.
If we find nothing for you to go on or the problem has disappeared i will close this issue.

Are you ok with that?

Grüße Ulf

<3

<!-- gh-comment-id:358108619 --> @ulfgebhardt commented on GitHub (Jan 16, 2018): Hey Jason, I have this issue particular when i try to reply to my own EMails. Maybe its address-book related. For other mails it works fine. I'm quite busy at the moment; as soon as i get the time i will update cypht again and make some checks. If we find nothing for you to go on or the problem has disappeared i will close this issue. Are you ok with that? Grüße Ulf <3
Author
Owner

@jasonmunro commented on GitHub (Jan 16, 2018):

This could definitely be a clue. IIRC, there is some logic to exclude your own addresses (the ones you have profiles for) from replies, I bet it's related to this. I think I should revisit that logic, and rather than exclude addresses, make sure we use all of them, but none are doubled up (I have seen reply all incorrectly have an address in both the To and CC fields).

Happy to keep the ticket open as long as we need to. I will update once I have a chance to check this particular code path. Thanks for the feedback!

<!-- gh-comment-id:358122147 --> @jasonmunro commented on GitHub (Jan 16, 2018): This could definitely be a clue. IIRC, there is some logic to exclude your own addresses (the ones you have profiles for) from replies, I bet it's related to this. I think I should revisit that logic, and rather than exclude addresses, make sure we use all of them, but none are doubled up (I have seen reply all incorrectly have an address in both the To and CC fields). Happy to keep the ticket open as long as we need to. I will update once I have a chance to check this particular code path. Thanks for the feedback!
Author
Owner

@jasonmunro commented on GitHub (Jan 16, 2018):

@ulfgebhardt the following patch can confirm or deny that we are inadvertently excluding your own E-mail address in replies which is the root of the issue:

https://gist.github.com/jasonmunro/fec698b8f3d16bf7c75e0df9b80b69ac

I'm pretty sure this is what is happening. I'm going to modify this behavior to not skip any address unless we already have it in the set of addresses in To and CC on reply or reply-all.

<!-- gh-comment-id:358134633 --> @jasonmunro commented on GitHub (Jan 16, 2018): @ulfgebhardt the following patch can confirm or deny that we are inadvertently excluding your own E-mail address in replies which is the root of the issue: https://gist.github.com/jasonmunro/fec698b8f3d16bf7c75e0df9b80b69ac I'm pretty sure this is what is happening. I'm going to modify this behavior to not skip any address unless we already have it in the set of addresses in To and CC on reply or reply-all.
Author
Owner

@ulfgebhardt commented on GitHub (Jan 21, 2018):

Giving Feedback: The Patch works like a charm - Reply & Reply all work as expected again.
Also the Return-Path is no longer used - all fine with the proposed changes.

I edited the file - this change is not yet in the Repo!

Rev: 34d70a0240

Grüße Ulf

<3

<!-- gh-comment-id:359244255 --> @ulfgebhardt commented on GitHub (Jan 21, 2018): Giving Feedback: The Patch works like a charm - Reply & Reply all work as expected again. Also the Return-Path is no longer used - all fine with the proposed changes. I edited the file - this change is not yet in the Repo! Rev: 34d70a0240c10b6b8ab9fedf7c96cf8a0ef0ed53 Grüße Ulf <3
Author
Owner

@jasonmunro commented on GitHub (Jan 21, 2018):

I did not commit that because I wanted to also refactor a few things. Fix is pushed in github.com/jasonmunro/cypht@5eca7950a1

This disables the "exclude my own addresses" logic, and also makes sure we never add the same E-mail to a reply all more than once.

<!-- gh-comment-id:359276763 --> @jasonmunro commented on GitHub (Jan 21, 2018): I did not commit that because I wanted to also refactor a few things. Fix is pushed in https://github.com/jasonmunro/cypht/commit/5eca7950a137b850a85eb0ad1aea51fee33cf9ae This disables the "exclude my own addresses" logic, and also makes sure we never add the same E-mail to a reply all more than once.
Author
Owner

@ulfgebhardt commented on GitHub (Jan 21, 2018):

Can confirm the "include my own address" logic ;-). I see what you wanted to do there.
I am still on the commit before tho.

Grüße Ulf

<3

<!-- gh-comment-id:359283518 --> @ulfgebhardt commented on GitHub (Jan 21, 2018): Can confirm the "include my own address" logic ;-). I see what you wanted to do there. I am still on the commit before tho. Grüße Ulf <3
Author
Owner

@ulfgebhardt commented on GitHub (Jan 22, 2018):

Confirming Fix in Rev 4e16293

Thanks, great work!

Grüße Ulf

<3

<!-- gh-comment-id:359560033 --> @ulfgebhardt commented on GitHub (Jan 22, 2018): Confirming Fix in Rev 4e16293 Thanks, great work! Grüße Ulf <3
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/cypht#213
No description provided.