Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Public View on MW 1.27.1 #10

Closed
tazinator opened this issue Oct 17, 2016 · 5 comments
Closed

Public View on MW 1.27.1 #10

tazinator opened this issue Oct 17, 2016 · 5 comments

Comments

@tazinator
Copy link

MW 1.27.1 is throwing the following error on the backend when any public user tries to view the wiki and isnt logged in to an SMF account.

PHP Catchable fatal error: Method User::__toString() must return a string value in .../includes/user/User.php on line 3747

Basically throws a 500 and no anonymous users can load any Wiki pages when the integration is enabled. Works just fine for logged in users, just not public.

@jdarwood007
Copy link
Member

jdarwood007 commented Oct 17, 2016

Are you using the Latest commit (f096e19 on Jul 6, 2014)?

We initially had an issue with our wiki on upgrading to 1.27.1. The only change I can see in our User.php is this:

Zeus:Downloads jeremy$ diff -u original/includes/user/User.php modified/includes/user/User.php
--- original/includes/user/User.php 2016-08-22 13:53:01.000000000 -0700
+++ modified/includes/user/User.php 2016-09-28 17:22:28.000000000 -0700
@@ -3744,7 +3744,7 @@
        } elseif ( !$session->getUser()->equals( $this ) ) {
            \MediaWiki\Logger\LoggerFactory::getInstance( 'session' )
                ->warning( __METHOD__ .
-                   ": Cannot log user \"$this\" out of a user \"{$session->getUser()}\"'s session"
+                   ": Cannot log user \"this\" out of a user \"{$session->getUser()}\"'s session"
                );
            // But we still may as well make this user object anon
            $this->clearInstanceCache( 'defaults' );

Most likely this to me seems like a bug as they shouldn't be using $this since its a special reference to the current object. I hadn't debugged anymore at the time.

@jdarwood007
Copy link
Member

jdarwood007 commented Oct 17, 2016

Actually looking at it more, it seems that the ip function isn't returning a string for some reason.

When you use $this in a string, it gets handled by the magic function __toString().

    /**
     * @return string
     */
    public function __toString() {
        return $this->getName();
    }

Which of course is handled by getName.

    /**
     * Get the user name, or the IP of an anonymous user
     * @return string User's name or IP address
     */
    public function getName() {
        if ( $this->isItemLoaded( 'name', 'only' ) ) {
            // Special case optimisation
            return $this->mName;
        } else {
            $this->load();
            if ( $this->mName === false ) {
                // Clean up IPs
                $this->mName = IP::sanitizeIP( $this->getRequest()->getIP() );
            }
            return $this->mName;
        }
    }

As it is a guest, it should return their IP

However the IP function has 2 places where a non string could be returned

    public static function sanitizeIP( $ip ) {
        $ip = trim( $ip );
        if ( $ip === '' ) {
            return null;
        }
        if ( self::isIPv4( $ip ) || !self::isIPv6( $ip ) ) {
            return $ip; // nothing else to do for IPv4 addresses or invalid ones
        }

If $ip was null or if it somehow doesn't fall into a valid ipv4 or ipv6 address. Only way to test this at this point would be to see what $this->getRequest()->getIP() is returning.

@tazinator
Copy link
Author

Just as a shot, changed my user/User.php to reflect what you guys have and it seems to have resolved the issue.

Changing line 3747 to:

": Cannot log user "this" out of a user "{$session->getUser()}"'s session"

@legoktm
Copy link

legoktm commented Oct 18, 2016

I filed https://phabricator.wikimedia.org/T148486 upstream for this.

@jdarwood007 jdarwood007 added this to the SMF 2.0 milestone Oct 18, 2016
@jdarwood007
Copy link
Member

This was fixed by MediaWiki and I haven't had any additional reports on this since then. Closing for now, if it still occurs we can reinvestigate this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants