roundcube / roundcubemail

The Roundcube Webmail suite
https://roundcube.net
GNU General Public License v3.0
5.88k stars 1.64k forks source link

v1.4.4 changes the way markasjunk calls its drivers' init() method #7361

Closed gurnec closed 4 years ago

gurnec commented 4 years ago

In v1.4.3, inside the init() method of a markasjunk driver, rcmail::get_instance()->output->type would always be 'html'. In v1.4.4 this has changed. This means that drivers which assumed it would be 'html' can fail. The demo jsevent.php driver is one such example:

https://github.com/roundcube/roundcubemail/blob/aadb13e25f73d783f731a99f9b9c2ea43bb10c79/plugins/markasjunk/drivers/jsevent.php#L68-L69

When ->output->type is 'json', it fails with the stack trace below since that ->output as no add_script() method.

Stack trace ``` Apr 30 17:03:22 hostname roundcube: PHP Fatal error: Uncaught Error: Call to undefined method rcmail_output_json::add_script() in /path/roundcubemail-1.4.4/plugins/markasjunk/drivers/jsevent.php:69 Stack trace: #0 /path/roundcubemail-1.4.4/plugins/markasjunk/markasjunk.php(341): markasjunk_jsevent->init() #1 /path/roundcubemail-1.4.4/plugins/markasjunk/markasjunk.php(130): markasjunk->_init_driver() #2 /path/roundcubemail-1.4.4/program/lib/Roundcube/rcube_plugin_api.php(100): markasjunk->init() #3 /path/roundcubemail-1.4.4/program/include/rcmail.php(137): rcube_plugin_api->init(Object(rcmail), 'mail') #4 /path/roundcubemail-1.4.4/program/include/rcmail.php(78): rcmail->startup() #5 /path/roundcubemail-1.4.4/index.php(43): rcmail::get_instance(0, NULL) #6 {main} thrown in /path/roundcubemail-1.4.4/plugins/markasjunk/drivers/jsevent.php on line 69 ```

I think this happened in this commit here when the call to initialize the driver was moved outside the end of that preceding if block. If I move it back up, it seems to revert to the previous behavior.

I'm not sure if this is even a bug, and if it is, if the above fix is correct. Pinging @johndoh since he authored that commit.

Thanks!

johndoh commented 4 years ago

try adding an output type check into the init() method of the jsevent driver.

--- a/plugins/markasjunk/drivers/jsevent.php
+++ b/plugins/markasjunk/drivers/jsevent.php
@@ -34,6 +34,10 @@ class markasjunk_jsevent

     public function init()
     {
+        $rcmail = rcmail::get_instance();
+        if ($rcmail->output->type != 'html')
+            return;
+
         $js_addition_spam_folders = json_encode($this->addition_spam_folders);
         $js_suspicious_folders    = json_encode($this->suspicious_folders);

The purpose of moving the init() call in the markasjunk class is to ensure a driver's init() method is always called when the driver is used.

gurnec commented 4 years ago

@johndoh Thanks for the quick response. Leaving this open to be closed by your PR.

alecpl commented 4 years ago

Thanks. Fixed.