Last modified: 2008-02-07 10:25:20 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T14270, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 12270 - GlobalAuth.php code review
GlobalAuth.php code review
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
General/Unknown (Other open bugs)
unspecified
All All
: Normal trivial (vote)
: ---
Assigned To: Nobody - You can work on this!
: patch, patch-need-review
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-12-10 18:56 UTC by Alno
Modified: 2008-02-07 10:25 UTC (History)
2 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
patch supposed to correct the bug found (1.72 KB, patch)
2007-12-10 18:56 UTC, Alno
Details

Description Alno 2007-12-10 18:56:33 UTC
Created attachment 4431 [details]
patch supposed to correct the bug found

Bug found while checking the code with Eclipse 3.3
Comment 1 Raimond Spekking 2008-02-06 06:18:27 UTC
Could you please check if

@@ -92,7 +92,7 @@
 			$fname );
 		while( $row = $dbr->fetchObject( $res ) ) {
 			if ( $row->user_wiki == $this->thiswiki || $row->user_wiki == '*' ) {
-				if ( $row->user_password == wfEncryptPassword( $row->$user_id, $password ) ) {
+				if ( $row->user_password == wfEncryptPassword( $row->userId, $password ) ) {


is really right? It's inconsistent to line 279.

Thanks.
Comment 2 Robert Leverington 2008-02-06 09:33:40 UTC
This cannot be (In reply to comment #1)
> Could you please check if
> @@ -92,7 +92,7 @@
>                         $fname );
>                 while( $row = $dbr->fetchObject( $res ) ) {
>                         if ( $row->user_wiki == $this->thiswiki ||
> $row->user_wiki == '*' ) {
> -                               if ( $row->user_password == wfEncryptPassword(
> $row->$user_id, $password ) ) {
> +                               if ( $row->user_password == wfEncryptPassword(
> $row->userId, $password ) ) {
> is really right? It's inconsistent to line 279.
> Thanks.

This cannot be correct given that the user id column in the user table is user_id not userId, therefore this line should be changed to

 if ( $row->user_password == wfEncryptPassword( $row->user_id, $password ) ) {

Simply removing the second $ from $row->$user_id would fix this problem - although I believe that this is already acceptable syntax anyway, but better be on the safe side and probably generates some sort of error.  Keeping second $ symbols out of object variable references also helps standardise code, but even so this may be a very minor issue.
Comment 3 Raimond Spekking 2008-02-07 10:25:20 UTC
Modified per comment #2 and applied with r30660.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links