From d0d39b51b8b85a86d8d62a4327d2f119d003cb78 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Wed, 5 Jan 2011 12:26:20 -0800 Subject: [PATCH 1/3] Fixes for RegisterThrottle plugin: alt registration methods (OpenID, FBConnect, Twitter) weren't triggering the throttle check or recording of IPs. Added StartRegistrationTry/EndRegistrationTry calls into those three, and moved the actual recording hook to EndUserRegister which is guaranteed to be called from User::register (so we don't need to worry about other auth methods forgetting to call the other UI-code hooks). --- plugins/Facebook/FBConnectAuth.php | 6 ++++++ plugins/OpenID/finishopenidlogin.php | 7 +++++++ .../RegisterThrottle/RegisterThrottlePlugin.php | 16 ++++++---------- plugins/TwitterBridge/twitterauthorization.php | 6 ++++++ 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/plugins/Facebook/FBConnectAuth.php b/plugins/Facebook/FBConnectAuth.php index d6d3786261..33c975d650 100644 --- a/plugins/Facebook/FBConnectAuth.php +++ b/plugins/Facebook/FBConnectAuth.php @@ -232,6 +232,10 @@ class FBConnectauthAction extends Action function createNewUser() { + if (!Event::handle('StartRegistrationTry', array($this))) { + return; + } + if (common_config('site', 'closed')) { // TRANS: Client error trying to register with registrations not allowed. $this->clientError(_m('Registration not allowed.')); @@ -300,6 +304,8 @@ class FBConnectauthAction extends Action common_debug('Facebook Connect Plugin - ' . "Registered new user $user->id from Facebook user $this->fbuid"); + Event::handle('EndRegistrationTry', array($this)); + common_redirect(common_local_url('showstream', array('nickname' => $user->nickname)), 303); } diff --git a/plugins/OpenID/finishopenidlogin.php b/plugins/OpenID/finishopenidlogin.php index 01dd61edb1..6c4e713897 100644 --- a/plugins/OpenID/finishopenidlogin.php +++ b/plugins/OpenID/finishopenidlogin.php @@ -247,6 +247,10 @@ class FinishopenidloginAction extends Action { # FIXME: save invite code before redirect, and check here + if (!Event::handle('StartRegistrationTry', array($this))) { + return; + } + if (common_config('site', 'closed')) { // TRANS: OpenID plugin message. No new user registration is allowed on the site. $this->clientError(_m('Registration not allowed.')); @@ -362,6 +366,9 @@ class FinishopenidloginAction extends Action common_rememberme($user); } unset($_SESSION['openid_rememberme']); + + Event::handle('EndRegistrationTry', array($this)); + common_redirect(common_local_url('showstream', array('nickname' => $user->nickname)), 303); } diff --git a/plugins/RegisterThrottle/RegisterThrottlePlugin.php b/plugins/RegisterThrottle/RegisterThrottlePlugin.php index 0078d3c600..e3982427da 100644 --- a/plugins/RegisterThrottle/RegisterThrottlePlugin.php +++ b/plugins/RegisterThrottle/RegisterThrottlePlugin.php @@ -167,28 +167,24 @@ class RegisterThrottlePlugin extends Plugin } /** - * Called after someone registers. + * Called after someone registers, by any means. * * We record the successful registration and IP address. * - * @param Action $action Action that is being executed + * @param Profile $profile new user's profile + * @param User $user new user * * @return boolean hook value * */ - function onEndRegistrationTry($action) + function onEndUserRegister($profile, $user) { $ipaddress = $this->_getIpAddress(); if (empty($ipaddress)) { - throw new ServerException(_m('Cannot find IP address.')); - } - - $user = common_current_user(); - - if (empty($user)) { - throw new ServerException(_m('Cannot find user after successful registration.')); + // User registration can happen from command-line scripts etc. + return true; } $reg = new Registration_ip(); diff --git a/plugins/TwitterBridge/twitterauthorization.php b/plugins/TwitterBridge/twitterauthorization.php index 931a037230..33ef71fcdd 100644 --- a/plugins/TwitterBridge/twitterauthorization.php +++ b/plugins/TwitterBridge/twitterauthorization.php @@ -419,6 +419,10 @@ class TwitterauthorizationAction extends Action function createNewUser() { + if (!Event::handle('StartRegistrationTry', array($this))) { + return; + } + if (common_config('site', 'closed')) { $this->clientError(_m('Registration not allowed.')); return; @@ -492,6 +496,8 @@ class TwitterauthorizationAction extends Action common_debug('TwitterBridge Plugin - ' . "Registered new user $user->id from Twitter user $this->twuid"); + Event::handle('EndRegistrationTry', array($this)); + common_redirect(common_local_url('showstream', array('nickname' => $user->nickname)), 303); } From 0ec07e9c651f747d93865f75b8631899a7d7f04d Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 7 Jan 2011 14:48:40 -0800 Subject: [PATCH 2/3] Use ReflectionFunction to check for a present-but-disabled dl() function instead of manually parsing the disable_functions php.ini setting. We were checking the list as comma-delimited (per the description of it as comma-delimited), but in fact spaces are also accepted, and who knows what else. --- lib/common.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/common.php b/lib/common.php index cd4fbfb15a..3963402158 100644 --- a/lib/common.php +++ b/lib/common.php @@ -60,8 +60,13 @@ if (!function_exists('dl')) { // Fortunately trying to call the disabled one will only trigger // a warning, not a fatal, so it's safe to leave it for our case. // Callers will be suppressing warnings anyway. - $disabled = array_filter(array_map('trim', explode(',', ini_get('disable_functions')))); - if (!in_array('dl', $disabled)) { + try { + // Reading the ini setting is hard as we don't know PHP's parsing, + // but we can check if it is disabled through reflection. + $dl = new ReflectionFunction('dl'); + // $disabled = $dl->isDisabled(); // don't need to check this now + } catch (ReflectionException $e) { + // Ok, it *really* doesn't exist! function dl($library) { return false; } From ea3105140140bc4537f34042b7c360619f5022b2 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Tue, 18 Jan 2011 11:08:32 -0800 Subject: [PATCH 3/3] TwitterBridge fix: merge down remaining 64-bit Snowflake ID fixes for twitterstatusfetcher.php from 0.9.x Original fixes in c169dcb5221cf3dd452c291bf97374bb459cc5b9; didn't get merged in 39cad55711897323fac5f651c003c4d815a51ae0 because the code had been broken out to another file, but manual merge went smooth. These affect twitterstatusfetcher.php on all 32-bit installs and some 64-bit installs (depending on whether the version of the JSON library reads the large numbers as long or double internally). 64-bit bug is harder to see as it tends to manifest as off-by-one due to losing a bit of precision off the end. --- .../daemons/twitterstatusfetcher.php | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/plugins/TwitterBridge/daemons/twitterstatusfetcher.php b/plugins/TwitterBridge/daemons/twitterstatusfetcher.php index 1b9cca8ecb..64b8acebde 100755 --- a/plugins/TwitterBridge/daemons/twitterstatusfetcher.php +++ b/plugins/TwitterBridge/daemons/twitterstatusfetcher.php @@ -200,7 +200,7 @@ class TwitterStatusFetcher extends ParallelizingDaemon if (preg_match("/$source/", mb_strtolower($status->source))) { common_debug($this->name() . ' - Skipping import of status ' . - $status->id . ' with source ' . $source); + twitter_id($status) . ' with source ' . $source); continue; } @@ -238,23 +238,24 @@ class TwitterStatusFetcher extends ParallelizingDaemon return null; } - $statusUri = $this->makeStatusURI($status->user->screen_name, $status->id); + $statusId = twitter_id($status); + $statusUri = $this->makeStatusURI($status->user->screen_name, $statusId); // check to see if we've already imported the status - $n2s = Notice_to_status::staticGet('status_id', $status->id); + $n2s = Notice_to_status::staticGet('status_id', $statusId); if (!empty($n2s)) { common_log( LOG_INFO, $this->name() . - " - Ignoring duplicate import: {$status->id}" + " - Ignoring duplicate import: {$statusId}" ); return Notice::staticGet('id', $n2s->notice_id); } // If it's a retweet, save it as a repeat! if (!empty($status->retweeted_status)) { - common_log(LOG_INFO, "Status {$status->id} is a retweet of {$status->retweeted_status->id}."); + common_log(LOG_INFO, "Status {$statusId} is a retweet of " . twitter_id($status->retweeted_status) . "."); $original = $this->saveStatus($status->retweeted_status); if (empty($original)) { return null; @@ -278,7 +279,7 @@ class TwitterStatusFetcher extends ParallelizingDaemon 'uri' => $statusUri, 'is_local' => Notice::GATEWAY)); common_log(LOG_INFO, "Saved {$repeat->id} as a repeat of {$original->id}"); - Notice_to_status::saveNew($repeat->id, $status->id); + Notice_to_status::saveNew($repeat->id, $statusId); return $repeat; } } @@ -297,17 +298,18 @@ class TwitterStatusFetcher extends ParallelizingDaemon $notice->reply_to = null; - if (!empty($status->in_reply_to_status_id)) { - common_log(LOG_INFO, "Status {$status->id} is a reply to status {$status->in_reply_to_status_id}"); - $n2s = Notice_to_status::staticGet('status_id', $status->in_reply_to_status_id); + $replyTo = twitter_id($status, 'in_reply_to_status_id'); + if (!empty($replyTo)) { + common_log(LOG_INFO, "Status {$statusId} is a reply to status {$replyTo}"); + $n2s = Notice_to_status::staticGet('status_id', $replyTo); if (empty($n2s)) { - common_log(LOG_INFO, "Couldn't find local notice for status {$status->in_reply_to_status_id}"); + common_log(LOG_INFO, "Couldn't find local notice for status {$replyTo}"); } else { $reply = Notice::staticGet('id', $n2s->notice_id); if (empty($reply)) { - common_log(LOG_INFO, "Couldn't find local notice for status {$status->in_reply_to_status_id}"); + common_log(LOG_INFO, "Couldn't find local notice for status {$replyTo}"); } else { - common_log(LOG_INFO, "Found local notice {$reply->id} for status {$status->in_reply_to_status_id}"); + common_log(LOG_INFO, "Found local notice {$reply->id} for status {$replyTo}"); $notice->reply_to = $reply->id; $notice->conversation = $reply->conversation; } @@ -317,7 +319,7 @@ class TwitterStatusFetcher extends ParallelizingDaemon if (empty($notice->conversation)) { $conv = Conversation::create(); $notice->conversation = $conv->id; - common_log(LOG_INFO, "No known conversation for status {$status->id} so making a new one {$conv->id}."); + common_log(LOG_INFO, "No known conversation for status {$statusId} so making a new one {$conv->id}."); } $notice->is_local = Notice::GATEWAY; @@ -338,7 +340,7 @@ class TwitterStatusFetcher extends ParallelizingDaemon Event::handle('EndNoticeSave', array($notice)); } - Notice_to_status::saveNew($notice->id, $status->id); + Notice_to_status::saveNew($notice->id, $statusId); $this->saveStatusMentions($notice, $status);