Aaron Schulz has uploaded a new change for review.
https://gerrit.wikimedia.org/r/258729
Change subject: Avoid "CAS updated failed" errors on Special:Preferences double
post
......................................................................
Avoid "CAS updated failed" errors on Special:Preferences double post
* This does the same thing ApiOptions does to avoid these errors.
A new getInstanceForUpdate() method is now in the User class to
simplify this pattern.
* Avoid overriding $user in ApiOptions for code readability.
* Fixed IDEA errors around Preferences::getFormObject() return type.
Bug: T95839
Change-Id: If2385b7486c043bd70d7031ff35e37dfb079a4d2
---
M includes/Preferences.php
M includes/api/ApiOptions.php
M includes/specials/SpecialPreferences.php
M includes/user/User.php
4 files changed, 33 insertions(+), 14 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/29/258729/1
diff --git a/includes/Preferences.php b/includes/Preferences.php
index c7ab9cd..7d1f35e 100644
--- a/includes/Preferences.php
+++ b/includes/Preferences.php
@@ -1274,7 +1274,7 @@
* @param IContextSource $context
* @param string $formClass
* @param array $remove Array of items to remove
- * @return HtmlForm
+ * @return PreferencesForm|HtmlForm
*/
static function getFormObject(
$user,
diff --git a/includes/api/ApiOptions.php b/includes/api/ApiOptions.php
index 74ce053..7a90527 100644
--- a/includes/api/ApiOptions.php
+++ b/includes/api/ApiOptions.php
@@ -35,14 +35,10 @@
* Changes preferences of the current user.
*/
public function execute() {
- $user = $this->getUser();
-
- if ( $user->isAnon() ) {
+ if ( $this->getUser()->isAnon() ) {
$this->dieUsage( 'Anonymous users cannot change
preferences', 'notloggedin' );
- }
-
- if ( !$user->isAllowed( 'editmyoptions' ) ) {
- $this->dieUsage( 'You don\'t have permission to edit
your options', 'permissiondenied' );
+ } elseif ( !$this->getUser()->isAllowed( 'editmyoptions' ) ) {
+ $this->dieUsage( "You don't have permission to edit
your options", 'permissiondenied' );
}
$params = $this->extractRequestParams();
@@ -53,11 +49,9 @@
}
// Load the user from the master to reduce CAS errors on double
post (T95839)
- if ( wfGetLB()->getServerCount() > 1 ) {
- $user = User::newFromId( $user->getId() );
- if ( !$user->loadFromId( User::READ_EXCLUSIVE ) ) {
- $this->dieUsage( 'Anonymous users cannot change
preferences', 'notloggedin' );
- }
+ $user = $this->getUser()->getInstanceForUpdate();
+ if ( !$user ) {
+ $this->dieUsage( 'Anonymous users cannot change
preferences', 'notloggedin' );
}
if ( $params['reset'] ) {
diff --git a/includes/specials/SpecialPreferences.php
b/includes/specials/SpecialPreferences.php
index 3b5b9c9..49ab6d5 100644
--- a/includes/specials/SpecialPreferences.php
+++ b/includes/specials/SpecialPreferences.php
@@ -65,7 +65,10 @@
$this->addHelpLink( 'Help:Preferences' );
- $htmlForm = Preferences::getFormObject( $this->getUser(),
$this->getContext() );
+ // Load the user from the master to reduce CAS errors on double
post (T95839)
+ $user = $this->getUser()->getInstanceForUpdate() ?:
$this->getUser();
+
+ $htmlForm = Preferences::getFormObject( $user,
$this->getContext() );
$htmlForm->setSubmitCallback( array( 'Preferences',
'tryUISubmit' ) );
$sectionTitles = $htmlForm->getPreferenceSections();
diff --git a/includes/user/User.php b/includes/user/User.php
index 3d1aa7e..84b8bdc 100644
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -5322,6 +5322,28 @@
}
/**
+ * Get a new instance of this user that was loaded from the master via
a locking read
+ *
+ * Use this instead of the main context User when updating that user.
This avoids races
+ * where that user was loaded from a slave or even the master but
without proper locks.
+ *
+ * @return User|null Returns null if the user was not found in the DB
+ * @since 1.27
+ */
+ public function getInstanceForUpdate() {
+ if ( !$this->getId() ) {
+ return null; // anon
+ }
+
+ $user = self::newFromId( $this->getId() );
+ if ( !$user->loadFromId( self::READ_EXCLUSIVE ) ) {
+ return null;
+ }
+
+ return $user;
+ }
+
+ /**
* Checks if two user objects point to the same user.
*
* @since 1.25
--
To view, visit https://gerrit.wikimedia.org/r/258729
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If2385b7486c043bd70d7031ff35e37dfb079a4d2
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
