GNUsocial/gnu-social
4
0
Fork 1
Code Issues Releases Wiki Activity

[DATABASE] Disable 'NULL' strings evaluation as SQL NULLs

Browse Source 
Use $object->sqlValue('NULL') (identical to DataObject_Cast'ing) instead and
fix related issues like (email|sms)settings considering these NULLs as a
false positive for the E-Mail address still being set when it's been removed.

There could also be security implications to the now-disabled approach of
considering 'NULL' strings as SQL NULLs.
This commit is contained in:
Alexei Sorokin 2019-11-02 12:21:43 +03:00
parent d921f3dadb
commit 5bc1b8695e
10 changed files with 89 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
actions/emailsettings.php
View File

@ -92,7 +92,7 @@ class EmailsettingsAction extends SettingsAction
$this->element('legend', null, _('Email address'));
$this->hidden('token', common_session_token());
if ($user->email) {
if (!$user->isNull('email')) {
$this->element('p', array('id' => 'form_confirmed'), $user->email);
// TRANS: Form note in e-mail settings form.
$this->element('p', array('class' => 'form_note'), _('Current confirmed email address.'));
@ -163,7 +163,7 @@ class EmailsettingsAction extends SettingsAction
$this->elementStart('div', array('id' => 'emailincoming'));
if ($user->incomingemail) {
if (!$user->isNull('incomingemail')) {
$this->elementStart('p');
$this->element('span', 'address', $user->incomingemail);
// @todo XXX: Looks a little awkward in the UI.
@ -180,7 +180,7 @@ class EmailsettingsAction extends SettingsAction
}
$this->elementStart('p');
if ($user->incomingemail) {
if (!$user->isNull('incomingemail')) {
// TRANS: Instructions for incoming e-mail address input form, when an address has already been assigned.
$msg = _('Make a new email address for posting to; '.
'cancels the old one.');
@ -435,7 +435,7 @@ class EmailsettingsAction extends SettingsAction
}
$original = clone($user);
$user->email = DB_DataObject_Cast::sql('NULL');
$user->email = $user->sqlValue('NULL');
// Throws exception on failure. Also performs it within a transaction.
$user->updateWithKeys($original);
@ -458,7 +458,7 @@ class EmailsettingsAction extends SettingsAction
}
$orig = clone($user);
$user->incomingemail = DB_DataObject_Cast::sql('NULL');
$user->incomingemail = $user->sqlValue('NULL');
$user->emailpost = false;
// Throws exception on failure. Also performs it within a transaction.
$user->updateWithKeys($orig);

14
actions/smssettings.php
View File

@ -98,7 +98,7 @@ class SmssettingsAction extends SettingsAction
$this->element('legend', null, _('SMS address'));
$this->hidden('token', common_session_token());
if ($user->sms) {
if (!$user->isNull('sms')) {
$carrier = $user->getCarrier();
$this->element(
'p',
@ -170,13 +170,13 @@ class SmssettingsAction extends SettingsAction
}
$this->elementEnd('fieldset');
if ($user->sms) {
if (!$user->isNull('sms')) {
$this->elementStart('fieldset', ['id' => 'settings_sms_incoming_email']);
// XXX: Confused! This is about SMS. Should this message be updated?
// TRANS: Form legend for incoming SMS settings form.
$this->element('legend', null, _('Incoming email'));
if ($user->incomingemail) {
if (!$user->isNull('incomingemail')) {
$this->element('p', 'form_unconfirmed', $user->incomingemail);
$this->element(
'p',
@ -417,9 +417,9 @@ class SmssettingsAction extends SettingsAction
$original = clone($user);
$user->sms = DB_DataObject_Cast::sql('NULL');
$user->carrier = DB_DataObject_Cast::sql('NULL');
$user->smsemail = DB_DataObject_Cast::sql('NULL');
$user->sms = $user->sqlValue('NULL');
$user->carrier = $user->sqlValue('NULL');
$user->smsemail = $user->sqlValue('NULL');
// Throws exception on failure. Also performs it within a transaction.
$user->updateWithKeys($original);
@ -531,7 +531,7 @@ class SmssettingsAction extends SettingsAction
$orig = clone($user);
$user->incomingemail = DB_DataObject_Cast::sql('NULL');
$user->incomingemail = $user->sqlValue('NULL');
// Throws exception on failure. Also performs it within a transaction.
$user->updateWithKeys($orig);

2
classes/Conversation.php
View File

@ -106,7 +106,7 @@ class Conversation extends Managed_DataObject
common_random_hexstr(8)
);
// locally generated Conversation objects don't get static URLs stored
$conv->url = DB_DataObject_Cast::sql('NULL');
$conv->url = $conv->sqlValue('NULL');
}
// This insert throws exceptions on failure
$conv->insert();

44
classes/Managed_DataObject.php
View File

@ -283,6 +283,8 @@ abstract class Managed_DataObject extends Memcached_DataObject
* Memcached_DataObject doesn't have enough info to handle properly.
*
* @return array of strings
* @throws MethodNotImplementedException
* @throws ServerException
*/
public function _allCacheKeys()
{
@ -440,6 +442,32 @@ abstract class Managed_DataObject extends Memcached_DataObject
return intval($this->id);
}
/**
* Check whether the column is NULL in SQL
*
* @param string $key column property name
*
* @return bool
*/
public function isNull(string $key): bool
{
if (array_key_exists($key, get_object_vars($this))
&& is_null($this->$key)) {
// If there was no fetch, this is a false positive.
return true;
} elseif (is_object($this->$key)
&& $this->$key instanceof DB_DataObject_Cast
&& $this->$key->type === 'sql') {
// This is cast to raw SQL, let's see if it's NULL.
return (strcasecmp($this->$key->value, 'NULL') == 0);
} elseif (DB_DataObject::_is_null($this, $key)) {
// DataObject's NULL magic should be disabled,
// this is just for completeness.
return true;
}
return false;
}
/**
* WARNING: Only use this on Profile and Notice. We should probably do
* this with traits/"implements" or whatever, but that's over the top
@ -516,10 +544,20 @@ abstract class Managed_DataObject extends Memcached_DataObject
// do it in a transaction
$this->query('BEGIN');
$parts = array();
$parts = [];
foreach ($this->keys() as $k) {
if (strcmp($this->$k, $orig->$k) != 0) {
$parts[] = $k . ' = ' . $this->_quote($this->$k);
$v = $this->table()[$k];
if ($this->$k !== $orig->$k) {
if (is_object($this->$k) && $this->$k instanceof DB_DataObject_Cast) {
$value = $this->$k->toString($v, $this->getDatabaseConnection());
} elseif (DB_DataObject::_is_null($this, $k)) {
$value = 'NULL';
} elseif ($v & DB_DATAOBJECT_STR) { // if a string
$value = $this->_quote((string) $this->$k);
} else {
$value = (int) $this->$k;
}
$parts[] = "{$k} = {$value}";
}
}
if (count($parts) == 0) {

8
classes/Memcached_DataObject.php
View File

@ -972,9 +972,15 @@ class Memcached_DataObject extends Safe_DataObject
case 'date':
$vstr = "{$v->year} - {$v->month} - {$v->day}";
break;
case 'sql':
if (strcasecmp($v->value, 'NULL') == 0) {
// Very selectively handling NULLs.
$vstr = '';
break;
}
// fallthrough
case 'blob':
case 'string':
case 'sql':
case 'datetime':
case 'time':
// Low level exception. No need for i18n as discussed with Brion.

3
extlib/DB/DataObject.php
View File

@ -4168,8 +4168,7 @@ class DB_DataObject
$method = $value;
$value = func_get_arg(1);
}
//require_once 'DB/DataObject/Cast.php';
require_once 'Cast.php';
require_once 'DB/DataObject/Cast.php';
return call_user_func(array('DB_DataObject_Cast', $method), $value);
}

1
lib/util/default.php
View File

@ -74,6 +74,7 @@ $default =
'mirror' => null,
'utf8' => true,
'db_driver' => 'DB', # XXX: JanRain libs only work with DB
'disable_null_strings' => true, // 'NULL' can be harmful
'quote_identifiers' => true,
'type' => 'mysql',
'schemacheck' => 'runtime', // 'runtime' or 'script'

2
modules/Favorite/FavoriteModule.php
View File

@ -75,7 +75,7 @@ class FavoriteModule extends ActivityVerbHandlerModule
while ($user->fetch()) {
$user->setPref('email', 'notify_fave', $user->emailnotifyfav);
$orig = clone($user);
$user->emailnotifyfav = 'null'; // flag this preference as migrated
$user->emailnotifyfav = $user->sqlValue('NULL'); // flag this preference as migrated
$user->update($orig);
}
printfnq("DONE.\n");

8
plugins/OStatus/classes/FeedSub.php
View File

@ -480,7 +480,7 @@ class FeedSub extends Managed_DataObject
$this->sub_end = common_sql_date(time() + $lease_seconds);
} else {
// Backwards compatibility to StatusNet (PuSH <0.4 supported permanent subs)
$this->sub_end = DB_DataObject_Cast::sql('NULL');
$this->sub_end = $this->sqlValue('NULL');
}
$this->modified = common_sql_now();
@ -496,10 +496,10 @@ class FeedSub extends Managed_DataObject
{
$original = clone($this);
$this->secret = DB_DataObject_Cast::sql('NULL');
$this->secret = $this->sqlValue('NULL');
$this->sub_state = 'inactive';
$this->sub_start = DB_DataObject_Cast::sql('NULL');
$this->sub_end = DB_DataObject_Cast::sql('NULL');
$this->sub_start = $this->sqlValue('NULL');
$this->sub_end = $this->sqlValue('NULL');
$this->modified = common_sql_now();
return $this->update($original);

21
scripts/upgrade.php
View File

@ -62,6 +62,7 @@ function main()
fixupNoticeConversation();
initConversation();
fixupUserBadNulls();
fixupGroupURI();
if ($iterate_files) {
printfnq("Running file iterations:\n");
@ -121,6 +122,26 @@ function updateSchemaPlugins()
printfnq("DONE.\n");
}
function fixupUserBadNulls(): void
{
printfnq("Ensuring all users have no empty strings for NULLs...");
foreach (['email', 'incomingemail', 'sms', 'smsemail'] as $col) {
$user = new User();
$user->whereAdd("{$col} = ''");
if ($user->find()) {
while ($user->fetch()) {
$sql = "UPDATE {$user->escapedTableName()} SET {$col} = NULL "
. "WHERE id = {$user->id}";
$user->query($sql);
}
}
}
printfnq("DONE.\n");
}
function fixupNoticeConversation()
{
printfnq("Ensuring all notices have a conversation ID...");