Move Conversation creation out of insert() to allow URI setting

This means we import the URI string from remote instances to track their
conversations and are able to stitch together replies in a single thread.

We might have to try to avoid collisions so noone remotely can predict
conversation URIs which we generate on our server, causing a DoS kind of
problem.
This commit is contained in:
Mikael Nordfeldth 2014-11-27 14:27:41 +01:00
parent 8a4575ea76
commit e0d0a59706
2 changed files with 31 additions and 13 deletions

View File

@ -63,7 +63,7 @@ class Conversation extends Managed_DataObject
* *
* @return Conversation the new conversation DO * @return Conversation the new conversation DO
*/ */
static function create(Notice $notice) static function create(Notice $notice, $uri=null)
{ {
if (empty($notice->id)) { if (empty($notice->id)) {
throw new ServerException(_('Tried to create conversation for not yet inserted notice')); throw new ServerException(_('Tried to create conversation for not yet inserted notice'));
@ -71,7 +71,7 @@ class Conversation extends Managed_DataObject
$conv = new Conversation(); $conv = new Conversation();
$conv->created = common_sql_now(); $conv->created = common_sql_now();
$conv->id = $notice->id; $conv->id = $notice->id;
$conv->uri = sprintf('%s%s=%d:%s=%s', $conv->uri = $uri ?: sprintf('%s%s=%d:%s=%s',
TagURI::mint(), TagURI::mint(),
'noticeId', $notice->id, 'noticeId', $notice->id,
'objectType', 'thread'); 'objectType', 'thread');

View File

@ -607,10 +607,19 @@ class Notice extends Managed_DataObject
// unknown remote user - within a known conversation. // unknown remote user - within a known conversation.
if (empty($notice->conversation) and !empty($options['conversation'])) { if (empty($notice->conversation) and !empty($options['conversation'])) {
$conv = Conversation::getKV('uri', $options['conversation']); $conv = Conversation::getKV('uri', $options['conversation']);
if ($conv instanceof Conversation and $activity->time > $conv->created) { if ($conv instanceof Conversation) {
common_debug('Conversation stitched together from (probably) reply to unknown remote user. Activity creation time ('.$activity->time.') is greater than conversation creation time ('.$conv->created.').'); common_debug('Conversation stitched together from (probably) reply to unknown remote user. Activity creation time ('.$activity->time.') should maybe be compared to conversation creation time ('.$conv->created.').');
$notice->conversation = $conv->id; $notice->conversation = $conv->id;
} else {
// Conversation URI was not found, so we must create it. But we can't create it
// until we have a Notice ID because of the database layout...
$notice->tmp_conv_uri = $options['conversation'];
} }
} else {
// If we're not using the attached conversation URI let's remove it
// so we don't mistake ourselves later, when creating our own Conversation.
// This implies that the notice knows which conversation it belongs to.
$options['conversation'] = null;
} }
} }
@ -661,6 +670,15 @@ class Notice extends Managed_DataObject
try { try {
$notice->insert(); // throws exception on failure $notice->insert(); // throws exception on failure
// If it's not part of a conversation, it's
// the beginning of a new conversation.
if (empty($notice->conversation)) {
$orig = clone($notice);
// $act->context->conversation will be null if it was not provided
$conv = Conversation::create($notice, $options['conversation']);
$notice->conversation = $conv->id;
$notice->update($orig);
}
} catch (Exception $e) { } catch (Exception $e) {
// Let's test if we managed initial insert, which would imply // Let's test if we managed initial insert, which would imply
// failing on some update-part (check 'insert()'). Delete if // failing on some update-part (check 'insert()'). Delete if
@ -869,13 +887,21 @@ class Notice extends Managed_DataObject
try { try {
$stored->insert(); // throws exception on error $stored->insert(); // throws exception on error
$orig = clone($stored); // for updating later in this try clause
// If it's not part of a conversation, it's
// the beginning of a new conversation.
if (empty($stored->conversation)) {
// $act->context->conversation will be null if it was not provided
$conv = Conversation::create($stored, $act->context->conversation);
$stored->conversation = $conv->id;
}
$object = null; $object = null;
Event::handle('StoreActivityObject', array($act, $stored, $options, &$object)); Event::handle('StoreActivityObject', array($act, $stored, $options, &$object));
if (empty($object)) { if (empty($object)) {
throw new ServerException('No object from StoreActivityObject '.$stored->uri . ': '.$act->asString()); throw new ServerException('No object from StoreActivityObject '.$stored->uri . ': '.$act->asString());
} }
$orig = clone($stored);
$stored->object_type = ActivityUtils::resolveUri($object->getObjectType(), true); $stored->object_type = ActivityUtils::resolveUri($object->getObjectType(), true);
$stored->update($orig); $stored->update($orig);
} catch (Exception $e) { } catch (Exception $e) {
@ -2392,14 +2418,6 @@ class Notice extends Managed_DataObject
$changed = true; $changed = true;
} }
// If it's not part of a conversation, it's
// the beginning of a new conversation.
if (empty($this->conversation)) {
$conv = Conversation::create($this);
$this->conversation = $conv->id;
$changed = true;
}
if ($changed && $this->update($orig) === false) { if ($changed && $this->update($orig) === false) {
common_log_db_error($notice, 'UPDATE', __FILE__); common_log_db_error($notice, 'UPDATE', __FILE__);
// TRANS: Server exception thrown when a notice cannot be updated. // TRANS: Server exception thrown when a notice cannot be updated.