Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MailboxHandler crashed on empty quota value #347

Closed
technicalguru opened this issue Apr 16, 2020 · 19 comments
Closed

MailboxHandler crashed on empty quota value #347

technicalguru opened this issue Apr 16, 2020 · 19 comments
Labels

Comments

@technicalguru
Copy link

When you add a new mailbox to a domain using an empty quota field then the model/MailBoxHandler.php crashes.

Actual behaviour: edit.php page crashes (database entry seems to be created nevertheless)
Expected behaviour: Regard quota as -1 (non-existent or default) or complain about empty field

Please find a patch
MailboxHandler.txt
for this as attachment

@DavidGoodwin
Copy link
Member

DavidGoodwin commented Apr 16, 2020

what version of postfixadmin are you using?

master already has something a bit different from what your patch is based on -

    protected function beforestore() {
        if (isset($this->values['quota']) && $this->values['quota'] != -1 && is_numeric($this->values['quota'])) {
            $multiplier = Config::read_string('quota_multiplier');
            if ($multiplier == 0 || !is_numeric($multiplier)) { // or empty string, or null, or false...
                $multiplier = 1;
            }
            $this->values['quota'] = $this->values['quota'] * $multiplier; # convert quota from MB to bytes
        }

see : https://github.com/postfixadmin/postfixadmin/blob/master/model/MailboxHandler.php#L221

@technicalguru
Copy link
Author

technicalguru commented Apr 16, 2020

The code you were posting is the code delievered from last stable download - 3.2 (via official website). My patch adds another check for the empty quota value at the beginning of the function.

If master fixes this it would be helpful to publish the fix or update the download section if already done. I just checked a few hours ago.

@DavidGoodwin
Copy link
Member

The fix probably isn't in 3.2 .... yet.

@DavidGoodwin
Copy link
Member

#342

@DavidGoodwin
Copy link
Member

@technicalguru
Copy link
Author

Thanks! Any idea when you will have the release published?

@DavidGoodwin
Copy link
Member

if git's log is to be believed, we released 3.2.3 back in Sept 2019. Since then there have been a couple of bug fixes - postfixadmin-3.2.3...postfixadmin_3.2 which seem to be mostly quota related.

I'll bump the smarty variant bundled in, and do a 3.2.4 release soon.

@technicalguru
Copy link
Author

Thank you for the fast fix. Please check the official download repositories. They do not offer any release after 3.2 - so 3.2.1 -> 3.2.3 are missing

@DavidGoodwin
Copy link
Member

You're looking in the wrong place - us cool kids hang out at : https://github.com/smarty-php/smarty/releases !! :-)

@DavidGoodwin
Copy link
Member

@technicalguru
Copy link
Author

Thanks for the hint. Nevertheless, the releases must be searchable. At the moment they are not. Try out a Google search. Appropriate links/hints to the newest distribution channels are missing. (I noticed the tag before this report but was unable to find a download for it.)

Thanks again for maintaining this software. It makes life so much easier :)

@DavidGoodwin
Copy link
Member

Oh, you're referring to the postfixadmin releases page(s) e.g. on sourceforge? I know they're stale; it's the annoyance of being split over two project "homes".

@technicalguru
Copy link
Author

Yes. They point to github. but from there is no hint where to get the releases. Perhaps it shall be mentioned in README.md.

@DavidGoodwin
Copy link
Member

I've setup some sort of integration between github and sourceforge; so the next github release may magically get mirrored there. I'm not sure how well that'll work.

I have deleted some of the old 3.2.x files and uploaded the 3.2.4.tar.gz file onto sourceforge.

@technicalguru
Copy link
Author

3.2.4 release does not include the fix:

protected function beforestore() {

    if (isset($this->values['quota']) && $this->values['quota'] != -1) {
        $multiplier = Config::read('quota_multiplier');
        if ($multiplier == 0 || !is_numeric($multiplier)) { // or empty string, or null, or false...
            $multiplier = 1;
        }
        $this->values['quota'] = $this->values['quota'] * $multiplier; # convert quota from MB to bytes
    }

Downloaded from: https://github.com/postfixadmin/postfixadmin/archive/postfixadmin-3.2.4.tar.gz

@DavidGoodwin
Copy link
Member

sigh. That'll teach me .....

Happy with the commit above?

@technicalguru
Copy link
Author

technicalguru commented Apr 18, 2020

The patch itself seems to work in my testings :)
Will check the next download package to verify...haha - Don't worry. Can happen...

@DavidGoodwin
Copy link
Member

Hmm, I'd rather avoid creating multiple releases, just for 1 line changes. Are you able to just download the latest MailboxHandler from https://raw.githubusercontent.com/postfixadmin/postfixadmin/postfixadmin_3.2/model/MailboxHandler.php and check against that ?

@technicalguru
Copy link
Author

All good. I am using a patch file for my purposes at this point in time :) I will re-evaluate when the next release is available. So you don't need to make an extra release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants