Enforcing max. length of edit note

[toringe]

Before 1.1.6.6 the maximum length of a page edit summary was not enforced in the form using maxlength. This did not cause major issues because the field was correctly truncated when storing a revision in the database.

The original reporter of this ticket experienced a major problem with edit notes. However, this does not appear to be a Wikka bug but a version-specific MySQL issue. See this discussion for further information and suggested workaround:  http://bugs.mysql.com/bug.php?id=18908. [1268] should help avoid issues of this kind.

---

The two input fields for notes in the edit.php file have a "size" attribute, but that doesn't prevent the user from typing more than 50 chars into the fields. (As defined by MAX_EDIT_NOTE_LENGTH)

If the user types a bit more than 50 chars in the note, then this results in: Query failed: SQL statement (Data too long for column 'note' at row 1)

This bug is worse than it seems: The page is also completely removed and most of the history is gone. This can be avoided by clicking on the back button in your browser and click on "store" again. Without a note, or a shorter note of course.

If you're using the wiki as a public wiki, then this can be abused by people who wants to wreck it, as the history is gone.

I've added an easy workaround to my own wiki: After the type="text" I've added maxlength="'.MAX_EDIT_NOTE_LENGTH.'" on both the input fields for notes. But I don't know if that takes care of the entire problem.

Preview:

if ($this->config['require_edit_note'] != 2) //check if edit_notes are enabled
		{
			$preview_buttons .= '<input size="'.MAX_EDIT_NOTE_LENGTH.'" type="text" maxlength="'.MAX_EDIT_NOTE_LENGTH.'" name="note" value="'.$this->hsc_secure($note).'" '.$highlight_note.'/>'.LABEL_EDIT_NOTE.'<br />'."\n";
		}

Store:

if ($this->config['require_edit_note'] != 2) //check if edit_notes are enabled
		{
			$output .= '<input size="'.MAX_EDIT_NOTE_LENGTH.'" type="text" maxlength="'.MAX_EDIT_NOTE_LENGTH.'" name="note" value="'.$this->hsc_secure($note).'" '.$highlight_note.'/> '.LABEL_EDIT_NOTE.'<br />'."\n";
		}

[BrianKoontz]

What version of MySQL are you using? As of 1.1.6.6 (not yet released), the "note" field is declared as a varchar(100). My testing (using MySQL 4.1) indicates that the field is being truncated properly, with no loss of data, error messages, etc. The MySQL documentation seems to confirm this behavior:

If you assign a value to a CHAR or VARCHAR column that exceeds the column's maximum length, the value is truncated to fit. If the truncated characters are not spaces, a warning is generated.

[toringe]

Server version: 5.0.67-community-nt

Info about the web server:

• Apache/2.2.9 (Win32) PHP/5.2.6
• MySQL client version: 5.0.51a
• PHP extension: mysql

Sorry for the late reply. I thought I should reply to the mail.

[BrianKoontz]

I could not recreate the DB crash or loss of page history. However, it's probably prudent to enforce MAX_EDIT_NOTE_LENGTH (otherwise, why bother defining it?).

[BrianKoontz]

(In [1268]) Added maxlength attribute to notes field. Refs #826

[DarTar]

toringe, I was also unable to reproduce this bug with PHP 5.2.6 / MySQL 5.0.51b. My Wikka instance behaves as expected (see Brian's first comment). I am afraid we need more research to understand what was causing the problem on your server.

If this issue is not reproducible, I suggest we apply the patch as in [1268] but change the wording of the ticket that suggests a major vulnerability so far unconfirmed.

[toringe]

We are testing now as we speak. The server administrator is trying to alter the MySQL configuration and test without the maxlenght property. So far it still crashing. Just had to test if it's the case for brand new pages too, and yes it is:

Query failed: insert into missions_pages set tag = 'TestPage', time = now(), owner = 'TorInge', user = 'TorInge', note = '1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890', latest = 'Y', body = 'Testpage' (Data too long for column 'note' at row 1)

[DarTar]

I just tested your query in MySQL on an unmodified Wikka table, everything works as expected (row stored with truncated string). Can you try to do the same (run a direct query in the DB) and report the result? Thanks.

mysql> \u wikka
Database changed
mysql> insert into 1165test_pages set tag = 'TestPage', time = now(), owner = 'TorInge', user = 'TorInge', note = '1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890', latest = 'Y', body = 'Testpage';
Query OK, 1 row affected, 1 warning (0.00 sec)

mysql> SHOW WARNINGS;
+---------+------+-------------------------------------------+
| Level   | Code | Message                                   |
+---------+------+-------------------------------------------+
| Warning | 1265 | Data truncated for column 'note' at row 1 | 
+---------+------+-------------------------------------------+
1 row in set (0.00 sec)

[DarTar]

After some research, it looks like you may have hit a version-specific MySQL bug. Take a look at this discussion:  http://bugs.mysql.com/bug.php?id=18908

[toringe]

I have notified the administrator about the bug. I hope we can sort this one out.

I tested the sql statement directly in phpmyadmin and got the same error:

#1406 - Data too long for column 'note' at row 1

[DarTar]

I am changing the status and title of the ticket accordingly. This appears to be a low level MySQL issue, not a Wikka bug. The ticket now refers to the enforcement of the edit note length limit as defined in MAX_EDIT_NOTE_LENGTH, patched by Brian.

[DarTar]

tested, changing description

[BrianKoontz]

(In [1338]) Ported to trunk. Refs #826.

[DotMG]

(In [1640]) refs #826

Added substr() to limit length of editnote

[BrianKoontz]

Tested OK.

[BrianKoontz]

Changed milestone to 1.3.1