enhancement needed - vaction plugin is working with roundcube 1.4, but needs refactoring (see comments)

studiowena commented 5 years ago

Hi,

I have update roundcube to 1.4RC1 and when going to settings and vacation plugin it show me message with

FILE NOT FOUND the request resource was not found failed request: www.domain.com/?_task=settings&_action=plugin.vacation

is this problem with vacation plugin or roundcube or configuration? It was working with 1.3.7

studiowena commented 5 years ago

please let me know how to fix it to work with new roundcube

rplessl commented 5 years ago

newer tried it with roundcube 1.4

studiowena commented 5 years ago

it could be nice to make it work with new version, i dont know how to fix the problem

rplessl commented 5 years ago

it seems that @jfcherng was successful to port the old code to the new roundcube https://github.com/jfcherng/roundcube-plugin-vacation/commits/NTHU

Myself I don't use roundcube anymore...

studiowena commented 5 years ago

Could You tell me why You stopped using Roundcube? Some better software? BTW thanks for link ill check it :)

jfcherng commented 5 years ago

@rplessl That branch is specific only for the use of my school (I am a student). And unfortunately I was using tool to "fix" some coding-style "problems" so it's not obvious for what I modified and making a PR...

In my memory, there were just some scope problems need to be modified for RC1.4. Such as raise_error becomes rcube::raise_error, Q() becomes rcube::Q(), rep_specialchars_output() becomes rcube_utils::rep_specialchars_output().

As mentioned above, I believe this commit is the real key to support RC1.4 although the commit message is RC1.3.

@studiowena The RC's error log (there should be some PHP errors logged in there) would be useful for debugging what's actually happening.

jfcherng commented 5 years ago

So you may try @gabor-toth https://github.com/gabor-toth/roundcube-vacation-plugin/commits/master support RC1.4 or not first.

studiowena commented 5 years ago

@jfcherng Your version should work? I have error DB Error: [1046] No database selected but in configuration of vacation plugin i have dbase = "mailadmin"

jfcherng commented 5 years ago

Your version should work?

I am not sure since the method used in my server is setuid only so I have only patched for it but not other methods. I think you may try gabor-toth's master branch.

studiowena commented 5 years ago

@jfcherng ok TY, i was trying his version before and its only for 1.3 version.

jfcherng commented 5 years ago

DB Error: [1046] No database selected

In the logged file, is there any line/file information for where it's going wrong?

studiowena commented 5 years ago

Aug 30 17:05:00 srv roundcube: PHP 11. vacation->vacation_form() /var/www/webmail/program/include/rcmail_output_html.php:1306 Aug 30 17:05:00 srv roundcube: PHP 12. Virtual->init() /var/www/webmail/plugins/vacation/vacation.php:96 Aug 30 17:05:00 srv roundcube: <bup1b43b> [1] SELECT goto FROM postfixadmin.alias WHERE address='login@domain.com' AND goto='login@domain.com'; Aug 30 17:05:00 srv roundcube: <bup1b43b> [2] SELECT subject,body,active FROM vacation WHERE email='login@domain.com'; Aug 30 17:05:00 srv roundcube: <bup1b43b> DB Error: [1046] No database selected (SQL Query: SELECT subject,body,active FROM vacation

jfcherng commented 5 years ago

@studiowena This commit looks like related to your error message. https://github.com/jfcherng/roundcube-plugin-vacation/commit/9caf8adc6089d4c4b60fe66e6c7a83c2f7c91958#diff-3781b940701891f92dd3a6373adf8f34R51-R53

studiowena commented 5 years ago

@jfcherng but i've downloaded all Your vacation plugin so i have newest files i think

https://github.com/jfcherng/roundcube-plugin-vacation/blob/NTHU/lib/virtual.class.php is from 17 JUN

studiowena commented 5 years ago

@jfcherng how to download this changed file You showed me? And why in Your package this files is without this changes?

jfcherng commented 5 years ago

@studiowena

how to download this changed file You showed me?

It's https://raw.githubusercontent.com/jfcherng/roundcube-plugin-vacation/9caf8adc6089d4c4b60fe66e6c7a83c2f7c91958/lib/virtual.class.php

And why in Your package this files is without this changes?

it looks like some changes were "recovered" in https://github.com/jfcherng/roundcube-plugin-vacation/commit/d85df9a3684a6fd0fc9d48936234532f772bce60. which I think was a mistake.

Can you test with the lib/virtual.class.php from https://github.com/jfcherng/roundcube-plugin-vacation/commit/9caf8adc6089d4c4b60fe66e6c7a83c2f7c91958. I mean only lib/virtual.class.php uses the old one.

studiowena commented 5 years ago

@jfcherng i tested and it partial work for me i see only

Forward and auto-reply ::: login@domain.com
Forwarding settings

Keep a copy of the mail

i cant change vacation message

jfcherng commented 5 years ago

Can you provide your config.ini for this plugin? You can change sensitive information to arbitrary text. I just need to use it for reproducing problem locally.

studiowena commented 5 years ago

driver = "virtual"

dsn = "mysql://virtual_vacation:password@localhost;charset=utf8" transport = "autoreply.domain.com"

dbase = "mailadmin"

always_keep_copy = true

select_query = "SELECT goto FROM mailadmin.alias WHERE address='%e' AND goto='%e'" delete_query = "DELETE FROM mailadmin.alias WHERE domain='%d' AND address='%e'" insert_query = "INSERT INTO mailadmin.alias (domain,address,goto) VALUES ('%d','%e','%g')"

createvacationconf = false always_keep_message = true disable_forward = true

jfcherng commented 5 years ago

i tested and it partial work for me i see only Forward and auto-reply ::: login@domain.com Forwarding settings Keep a copy of the mail

Did this produce any error log in the log file?

jfcherng commented 5 years ago

I seem to be able to reproduce it now. May investigate later.

jfcherng commented 5 years ago

The first problem that I found is that I only check for dotforward driver/settings when show the plugin form. As I said the NTHU branch is modified to fit the usage for my school's server which is using dotforward and setuid.

You may force adding return true; to both functions so they look like this.

    // Helper method for the template to determine if user is allowed to enter aliases
    public final function useAliases()
    {
        return true;
        return (isset($this->dotforward['alias_identities']) && $this->dotforward['alias_identities']);
    }

    // Vacation auto reply is enabled? (jfcherng)
    final public function useVacationAutoReply()
    {
        return true;
        return isset($this->dotforward['binary']) && $this->dotforward['binary'] !== '';
    }

The form should shows and test for further problems if there is any.

studiowena commented 5 years ago

@jfcherng

now its working but in logs have some warnings Aug 30 22:20:58 srv roundcube: PHP Deprecated: Non-static method MDB2::parseDSN() should not be called statically, assuming $this from incompatible context in /var/www/webmail/plugins/vacation/lib/virtual.class.php on line 30

this warnings are only if vacation plugin is enabled

jfcherng commented 5 years ago

It had been fixed by the upstream (pear/MDB2) for a VERY LONG time ago (on 1 Mar 2010) in https://github.com/pear/MDB2/commit/81a0d4ae76b4c759cdb43a277f10863e346f6ba8#diff-95a0a48d62bb062e1cf9c0d7e1054bfbR794.

But if you are using old PHP (<7) or old operating system, the simplest fix would be suppress the warning via adding a @ for that line (/var/www/webmail/plugins/vacation/lib/virtual.class.php on line 30).

$dsn = @MDB2::parseDSN($this->cfg['dsn']);

I have tested the latest master branch of roundcube is able to be run on PHP 7.4-dev. Yeah, I run PHP 7.4-dev with roundcube master branch (with some personal patches which I do not think they will be merged) on the production server. I did not hear people complain about bugs yet for months.

jfcherng commented 5 years ago

By the way, you have better test the actually functionality. I.e., send a real mail to a on-vacation account to see if it works or not.

studiowena commented 5 years ago

Now its working without any warnings in logs :) and yes i get email notification when sending to account have vacation enabled :) I need to translate to my lang and fix some chracter set in mysql DB :)

i think we can make it as resolved :)

jfcherng commented 5 years ago

@rplessl The problem in the current branch is that RC 1.4 introduces a new RWD (responsive web design) skin named as elastic but in this plugin, RC cannot find a template for elastic from https://github.com/rplessl/roundcube-vacation-plugin/tree/2b81025c82b6c913f47483c659915a658de00a2c/skins.

In my NTHU branch, I deleted those template files and used RC's API to construct HTML. https://github.com/jfcherng/roundcube-plugin-vacation/blob/ccdfea09d4987804063e732ba87159c93ee3e015/vacation.php#L93-L247

But unfortunately, during modifying, I changed some localization keys. So that's why @studiowena has to re-translate to his lang, there is only en_US and zh_TW left in my branch.

rplessl commented 5 years ago

thanks for your all your infos and testing @jfcherng and @studiowena!

a warm hug to the one who has time and desire to expand and fix this for the vacation feature!

topoldo commented 4 years ago

Is there any possibility to have from @rplessl or from @jfcherng or from @studiowena a correct and complete version? I need to use the ftp as driver and I can contribute by redoing the Italian translation. Tnx in advance Topoldo

jfcherng commented 4 years ago

My NTHU branch may work with luck. I don't remember I modified ftp things so the driver part was probably kept as-is.

topoldo commented 4 years ago

Il giorno 15/11/2019 alle 17:00, Jack Cherng ha scritto:

My NTHU branch https://github.com/jfcherng/roundcube-plugin-vacation/tree/NTHU may work with luck. I don't remember I modified ftp things so the driver part is probably kept as-is.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rplessl/roundcube-vacation-plugin/issues/16?email_source=notifications&email_token=ACHML2KRGYXJUFRX5Z6ANALQT3BSZA5CNFSM4G6OXX6KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEF4CCI#issuecomment-554418441, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACHML2KNZQV4R2EE5L37ZJDQT3BSZANCNFSM4G6OXX6A.

many thanks! I'll try it. If it works, I'll provide you with the Italian translation. Cheers Leopoldo/Topoldo

-- Leopoldo Saggin MD PhD Via A. Memmo, 25 35122 PADOVA (PD) - Italy Phone: +39 049 875 2158 (home) Phone: +39 049 827 6016 (office) Email: leopoldo.saggin@yahoo.com Skype: leopoldo.saggin

Due cose riempiono la mente con sempre nuova e crescente ammirazione e rispetto, quanto più spesso e più a lungo la riflessione si occupa di esse: il cielo stellato sopra di me e la legge morale in me.

Zwei Dinge erfüllen das Gemüt mit immer neuer und zunehmender Bewunderung und Ehrfurcht, je öfter und anhaltender sich das Nachdenken damit beschäftigt: Der bestirnte Himmel über mir und das moralische Gesetz in mir.

                                                         I. Kant

topoldo commented 4 years ago

@jfcherng: many thanks for sending me your version of vacation. In my hands it seems not to work. With respect to the version you sent me, I only changed your config.ini with that I used with "my" vacation which worked in roundcube 1.3.9 beside, I setup the correct owner and group (ie www-data:www-data) to all its dirs and files. When I select "vacation" from roundcube settings, after enabling it intoconfig.inc.php it appears this on the screen: "Vacation plugin: Cannot login to FTP-server 'mail.bio.unipd.it' with username: leopoldo Please read the INSTALL instructions!"

My config.ini content is:

[default] driver = "ftp" passive = true <= I added this line to the original after my first unsuccessfull tries (however no luck!) subject = "Default subject" body ="default body"

[dotforward] binary = "/usr/bin/vacation" flags = "" message = ".vacation.msg" database = ".vacation.db" alias identities = true set_envelop_sender = false always_keep_message = true

Topoldo

jfcherng commented 4 years ago

Unfortunately I have never tried a FTP setup and I have no plan to try it now. So if it's not working then it's not working :(

topoldo commented 4 years ago

Tnx anyway! Topoldo

topoldo commented 4 years ago

I tried to use your version BUT iI substituted your lib/ftp.php.class with the "original" one. What I can say at the moment is that:

Autoreply works
.forward works too! :-)) I'm not intersted into alias so i'll do not test it but I suspec that there is no reason why it should not work Tnx again! Topoldo/Leopoldo

topoldo commented 4 years ago

In a short time I'll post my translation into Italian, if someone needs it. Topoldo

jfcherng commented 4 years ago

@topoldo

I tried to use your version BUT iI substituted your lib/ftp.php.class with the "original" one.

What does the "original" mean? The current master or latest stable?

topoldo commented 4 years ago

The version of Vacation I installed with the "standard" methods using composer.phar and asking for gabtoth/vacation inside roundcube 1.3.9

Herein thre is my localization into Italian

it_IT.inc.txt

Regards, Topoldo/Leopoldo

topoldo commented 4 years ago

If you want I can upload it. I'm not confident con github. Anyway, now I'm goingo heme so you can expect it for tomorrow! T/L

jfcherng commented 4 years ago

If it's done by composer without specific constraint, it's the latest stable. Thanks. I will replace my branch's with it.

topoldo commented 4 years ago

It is. T/L

studiowena commented 4 years ago

i've just back and read all :) great its working for You (im using DB driver only)

jfcherng commented 4 years ago

someone has seemed to make it https://github.com/bukowski12/roundcube-vacation-plugin/commit/55d4feffd340dbdc9b2dbd493d7482c628e16fed

rplessl commented 4 years ago

perfect, so please use that fork for further enhancements:

https://github.com/bukowski12/roundcube-vacation-plugin

I will archive my fork and development of this roundcube plugin

rplessl / roundcube-vacation-plugin

enhancement needed - vaction plugin is working with roundcube 1.4, but needs refactoring (see comments) #16