r3c / custom_from

Plugin for Roundcube webmail, enable virtual email sender input.
https://plugins.roundcube.net/#/packages/r3c/custom-from
Other
20 stars 16 forks source link

PHP Warning in message_compose_body() #22

Closed kochs-online closed 3 years ago

kochs-online commented 3 years ago

I noticed a bunch of PHP warnings in my RoundCube error log originating from custom_from:

PHP Warning:  Creating default object from empty value in [roundcube path]/plugins/custom_from/custom_from.php on line 172

Updating to https://github.com/r3c/custom_from/commit/c596a6dd4c0cbef360de28c64eba2360f66c1250 did not fix this issue.

I think it is caused by the first loop in function message_compose_body() when $message – and consequently $message->headers – has not been set yet.

This could be fixed by adding

if (!isset($message)) $message = new stdClass();
if (!isset($message->headers)) $message->headers = new stdClass();

to the beginning of the foreach loop.

r3c commented 3 years ago

Hi and thank you for this bug report!

I just checked my own logs and don't see this error: could you please tell me what version of Roundcube you're using? (I'm running on 1.4.8).

Based on your description of the error and related code here I guess the $params['message'] variable is not always defined, so I would rather early exit from the whole message_compose_body method in that case. What do you think?

kochs-online commented 3 years ago

I'm running RoundCube 1.4.6. Being a warning the log entry might only appear when setting error_reporting() accordingly.

var_dumping $params (at the beginning of message_compose_body()) when composing a new message I get this output:

array(4) {
  ["body"]=>
    NULL
  ["html"]=>
    bool(false)
  ["mode"]=>
    NULL
  ["abort"]=>
    bool(false)
}

I guess exiting message_compose_body() early might do the trick. If $params['message'] is not set, there is no address to be filtered, right?

r3c commented 3 years ago

This was my understanding from reading Roundcube source code, thank you for having a look too. I'll apply the fix you suggest 🙂

kochs-online commented 3 years ago

Yes, let's have a try with exiting early from message_compose_body(). If you commit a change, I'll test it and report back.

r3c commented 3 years ago

Just pushed 19c5b8e, but I also did some testing locally and didn't find a scenario when 'message' is null, so I hope this is the right fix for your issue. Please let me know more about it if you have some time to test!

r3c commented 3 years ago

Any news about this @kochs-online ? 🙂

emtiu commented 3 years ago

This may be related: I'm seeing the following error in my logs whenever I compose or reply to a message on roundcube 1.5-rc:

roundcube[34615]: PHP Error: $COMPOSE or $MESSAGE global variable is undefined, custom_from can't work properly in /usr/home/mbueker/Roundcube/plugins/custom_from/custom_from.php on line 161 (GET /?_task=mail&_action=compose&_id=152339252690f31913c477c)

However, despite this error message, custom_from works as expected (selecting identities from headers etc.).

r3c commented 3 years ago

Looks like a different issue, however I'm surprized custom_from works as expected when this message appears in logs, since it means part of the processing was interrupted (that would be the part that removes your custom address from recipients so you don't include yourself when replying). I'll have a look and see what changed in 1.5 RC.

r3c commented 3 years ago

Compatibility with Roundcube 1.5 was indeed broken. I just pushed a3f3710bd32e19a1a2bea578b1c498df7bea1336 that should hopefully fix the issue, but I only conducted basic validation so far. Let me know if you experience any issue with this version 🙂

emtiu commented 3 years ago

We're getting there :) With a3f3710, the error message is gone in the following case:

However, the error still appears (in its new form: missing state variable) in the following cases:

r3c commented 3 years ago

Thanks a lot for taking the time to do some testing @emtiu! You're right indeed, part of the error was based on wrong assumption. I just sent 95dec70619a1f9b9e11d6dc7673eeb9220ae45aa and tested it locally with 1.4.1 and 1.5-rc versions, both seem to be working fine now 🙂

emtiu commented 3 years ago

Looking good! All my test cases work as expected, and there's no more errors :) Thanks a lot!

r3c commented 3 years ago

Thank you for your help! I'll publish a new version with this fix included during the week 🙂

r3c commented 3 years ago

Released in https://github.com/r3c/custom_from/releases/tag/1.6.5 ; thanks again for your help!