updating group fails fatally under certain conditions

[Stefan Topfstedt]

General Info:
--------------
- Mahara 1.6.4
- PostgreSQL 8.4.12
- PHP 5.3.10
- RHEL 5.8

Browser Info:
---------------
- Chrome 26, Firefox 20 (Win), latest Safari on Mac as well.

Other:
--------
Mahara is configured with clean-URLs turned on.

Description:
--------------
Saving a group after editing fails.

After submitting the "Edit group" form, the user gets thrown to an error page. The error message reads:

    Mahara: Site unavailable
    A nonrecoverable error occurred. This probably means you have encountered a bug in the system

Apparently, Mahara is unable to recover from a failed attempt to update a record in the database. (please see attached error log).

This only happens under very specific conditions, and seems to be connected to clean URL mode.

To reproduce:

1. turn off clean URL mode in the Mahara configuration.
2. create a two groups (group A and group B). Observe that the "urlid" field in these records have NULL values.
3. turn clean URL mode back on.
4. open Group A for editing. Make a change, but leave the "Group homepage URL" in the input form empty. Hit the "save group" button.
5. open Group B for editing. Make a change, but leave the "Group homepage URL" in the input form empty as well. "Hit "save group". **error occurs**

[Stefan Topfstedt (stefan-topfstedt)]

Btw, will patch this some time in the next couple of days and then attach it to this issue.

[Stefan Topfstedt (stefan-topfstedt)]

Attached patch, please review. Thanks!

[Aaron Wells (u-aaronw)]

Hi Stefan,

Thanks for the patch! :)

Since you know git, please consider pushing your patch into our gerrit code review system. Instructions are here: https://wiki.mahara.org/index.php/Developer_Area/Contributing_Code

If you don't want to do that, I'd be happy to push the patch in for you. I just need to know what name and email to put on the commit's "author" line.

Cheers,
Aaron

[Stefan Topfstedt (stefan-topfstedt)]

Hi Aaron,

Thanks for the good info.

I pushed the change into Gerrit.

Please see https://reviews.mahara.org/2094

I also removed the (botched) patch that was attached to this ticket.

Cheers,

 -Stefan

[Aaron Wells (u-aaronw)]

Okay, I see what's going on. The group.urlid column in the database has a uniqueness constraint on it. But, it's also NULLable, and even with a uniqueness constraint in place, it's okay to have multiple rows with NULL values.

But, the way the code's written, when you activate cleanurl mode, and then make changes to an old group that still doesn't have a urlid value yet, the code saves it as the empty string '' rather than as NULL. These are more or less identical on the PHP side of things, but on the database side the difference is important because '' DOES count towards uniqueness. So, when you go to edit a second group without a urlid, the code tries to set its urlid to '' as well, and dies because it violates that uniqueness constraint.

One workaround to this problem is to run the "regenerate clean URLs" script immediately after activating cleanurls. But, in the absence of that, this patch will at least avoid an error condition. :)

It's quite possible there may be the same issue for Users and Pages. I think no one has reported it yet because most sites run the "regenerate clean URLs" script.

[Aaron Wells (u-aaronw)]

Ported to master and 1.7:

master: https://reviews.mahara.org/#/c/2124/
1.7: accidental direct commit rather than going through gerrit