[PHPBB3-14556] Disapproving posts with notification causes postgreSQL error (> 4,000 characters)

Discuss requests for comments/changes posted in the Issue Tracker for the development of phpBB. Current releases are 3.2/Rhea and 3.3/Proteus.
Locked
Lady_G
Registered User
Posts: 38
Joined: Sun Aug 31, 2014 3:02 pm

[PHPBB3-14556] Disapproving posts with notification causes postgreSQL error (> 4,000 characters)

Post by Lady_G »

When a moderator disapproves a post > 4000 characters with notification ([X] Notify poster about disapproval?), postgreSQL triggers an error:

Code: Select all

SQL ERROR [ postgres ]

ERROR: value too long for type character varying(4000) []

SQL

INSERT INTO notifications (item_id, notification_type_id, item_parent_id, notification_time, notification_read, notification_data, user_id) VALUES (184639, 14, 1, 1458606017, 0, 'a:5:{s:17:"disapprove_reason";
... , 490)
The moderator log records the disapproval, but no notification is sent. I created an issue in JIRA: [PHPBB3-14556] Disapproving posts with notification causes postgreSQL error (> 4,000 characters)

The problem is due to a limitation in the notifications table, column 'notification_data'. In phpbb/db/migration/data/v310/notifications.php,

Code: Select all

$this->table_prefix . 'notifications'		=> array(
	'COLUMNS'			=> array(
		'notification_id'  	=> array('UINT', null, 'auto_increment'),
		'item_type'		  => array('VCHAR:255', ''),
		'item_id'		  	  => array('UINT', 0),
		'item_parent_id'   	  => array('UINT', 0),
		'user_id'			 => array('UINT', 0),
		'notification_read'	=> array('BOOL', 0),
		'notification_time'	=> array('TIMESTAMP', 1),
		'notification_data'		=> array('TEXT_UNI', ''),
'notification_data' is defined as array('TEXT_UNI', ''). The postgres limit is defined in phpbb/db/tools.php:

Code: Select all

'postgres'	=> array(
...
	'TEXT'		=> 'varchar(8000)',
	'MTEXT'		=> 'TEXT',
...
	'TEXT_UNI'	=> 'varchar(4000)',
	'MTEXT_UNI'	=> 'TEXT',
...	),
I think 'TEXT_UNI' should be changed from 'varchar(4000)' to 'varchar(8000)'. The columns are set during the install / upgrade process, and I don't know how to test this. However, I have manually modified the database and can confirm this will fix the problem.

I had a similar problem with the Board Rules extension: PostgreSQL error: value too long for type character varying - Board Rules VSE fixed the problem by modifying the rule message to point to MTEXT_UNI, which has a limit of 8000 characters. (github commit)

Changing a database limit is not trivial. I do not want to make a change that (1) I don't know how to properly test and (2) will impact all functions that use 'TEXT_UNI'.

Can someone please provide guidance?

User avatar
david63
Registered User
Posts: 355
Joined: Mon Feb 07, 2005 7:23 am

Re: [PHPBB3-14556] Disapproving posts with notification causes postgreSQL error (> 4,000 characters)

Post by david63 »

In my view that is not the correct way to solve the problem. Increasing the size of the field means that the problem still exists, admittedly not for another 4k characters. If there is a size limit then it should be checked at the input stage and a controlled error given.

As an aside what are you saying about disapproving a post that takes so many characters? I doubt that I have ever gone over 100!
David
Remember: You only know what you know -
and you do not know what you do not know!

User avatar
MattF
Extension Customisations
Extension Customisations
Posts: 675
Joined: Mon Mar 08, 2010 9:18 am

Re: [PHPBB3-14556] Disapproving posts with notification causes postgreSQL error (> 4,000 characters)

Post by MattF »

Nope, TEXT and TEXT_UNI should not be changed. In these situations, 'notification_data' should be switched over from TEXT_UNI to MTEXT_UNI.

However, notification_data should not be trying to store > 4,000 characters of data, so that is what really needs to be looked at.

Like it does here: https://github.com/phpbb/phpbb/blob/3.1 ... #L443-L445
Has an irascible disposition.

Lady_G
Registered User
Posts: 38
Joined: Sun Aug 31, 2014 3:02 pm

Re: [PHPBB3-14556] Disapproving posts with notification causes postgreSQL error (> 4,000 characters)

Post by Lady_G »

david63 wrote: Thu Mar 24, 2016 7:29 am As an aside what are you saying about disapproving a post that takes so many characters? I doubt that I have ever gone over 100!
This is the critical point to answer.

I agree the disapproval message itself can be less than 100 characters, but my board tries to help people. As a courtesy, we always include the content of their submitted post with the disapproval message.

Not for spammers, certainly. But, there are certain cases where new members supply information that exceeds 4,000 characters.

The new board member will get an email with a record of what they submitted. They can use the email's content to resubmit a corrected post which will comply with our Board Rules.
VSE wrote: Thu Mar 24, 2016 7:13 pm Nope, TEXT and TEXT_UNI should not be changed. In these situations, 'notification_data' should be switched over from TEXT_UNI to MTEXT_UNI.

However, notification_data should not be trying to store > 4,000 characters of data, so that is what really needs to be looked at.

Like it does here: https://github.com/phpbb/phpbb/blob/3.1 ... #L443-L445
I did see see that statement, but it did not trigger the error. Perhaps the length of the serialized data exceeding 4000 characters could be truncated instead (without corrupting the data stream content)?

My intent is to avoid a postgres error when a moderator disapproves a post notification > 4,000 characters. Having a partial disapproval message would be better than none. The moderator log contains the full message content, which can be retrieved if needed.

User avatar
MattF
Extension Customisations
Extension Customisations
Posts: 675
Joined: Mon Mar 08, 2010 9:18 am

Re: [PHPBB3-14556] Disapproving posts with notification causes postgreSQL error (> 4,000 characters)

Post by MattF »

Lady_G wrote: Thu Mar 24, 2016 10:08 pmI did see see that statement, but it did not trigger the error. Perhaps the length of the serialized data exceeding 4000 characters could be truncated instead (without corrupting the data stream content)?

My intent is to avoid a postgres error when a moderator disapproves a post notification > 4,000 characters. Having a partial disapproval message would be better than none. The moderator log contains the full message content, which can be retrieved if needed.
The DBAL is correct, TEXT is supposed to be limited to 4,000 for postgresql.

The solution is to either switch the notification_data field to MTEXT_UNI, or do as the posts notification does, check (and trim) >4000 strings before saving to notification_data in the post_approve and post_disapprove notification methods, the latter of which I'm inclined to think is the proper solution.

I still don't get why an entire post would be saved to the notification data though, as that doesn't seem right to me.
Has an irascible disposition.

Lady_G
Registered User
Posts: 38
Joined: Sun Aug 31, 2014 3:02 pm

Re: [PHPBB3-14556] Disapproving posts with notification causes postgreSQL error (> 4,000 characters)

Post by Lady_G »

This is my first board as a moderator (and site admin), so I have no prior experience on what "seems right".

Perhaps my board operates differently than the majority of users, but the action of creating a disapproval notice > 4000 characters has triggered an error.

Asking a moderator to check the post length before disapproval is not appropriate, as this creates extra work. They may not check every disapproval, or, they may not know how to check the post length.
VSE wrote: Thu Mar 24, 2016 11:26 pm The solution is to either switch the notification_data field to MTEXT_UNI, or do as the posts notification does, check (and trim) >4000 strings before saving to notification_data in the post_approve and post_disapprove notification methods, the latter of which I'm inclined to think is the proper solution.
Another suggestion would be to implement trigger_error() to inform the moderator. The moderator can then click the browser "back" button to resubmit a revised notice, or close the browser tab and start again.

There is an error message in language/en (and language/en_us) posting.php: 'TOO_MANY_CHARS_LIMIT'

The database only sees the serialized data, which is larger than the notification message. Perhaps to replace return array(); with trigger_error('TOO_MANY_CHARS_LIMIT', 4000); in: phpbb/post.php?

Code: Select all

	$serialized_data = serialize($this->get_data(false));

	// If the data is longer then 4000 characters, it would cause a SQL error.
	// We don't add the username to the list if this is the case.
	if (utf8_strlen($serialized_data) >= 4000)
	{
		return array();
	}
(This is a concept to show the intent.)

User avatar
MattF
Extension Customisations
Extension Customisations
Posts: 675
Joined: Mon Mar 08, 2010 9:18 am

Re: [PHPBB3-14556] Disapproving posts with notification causes postgreSQL error (> 4,000 characters)

Post by MattF »

You're missing the point. The solution is for the php code to handle messages >4,000 chars differently. This will be transparent to users. Don't worry, the developers will fix this bug the correct way.
Has an irascible disposition.

Lady_G
Registered User
Posts: 38
Joined: Sun Aug 31, 2014 3:02 pm

Re: [PHPBB3-14556] Disapproving posts with notification causes postgreSQL error (> 4,000 characters)

Post by Lady_G »

In hindsight, I see that I did misunderstood your point. Thank you, I will wait for the correct solution.

Lady_G
Registered User
Posts: 38
Joined: Sun Aug 31, 2014 3:02 pm

Re: [PHPBB3-14556] Disapproving posts with notification causes postgreSQL error (> 4,000 characters)

Post by Lady_G »

Has there been any further development to handle disapproval messages > 4,000 characters?

Locked