1.6-Polygot

[huangzj]

The plugin doesn't get activated in version 1.6-Polygot of YOURLS. Will it be updated to support this version?

[ianbarber]

Ouch, think we've hit a slightly tricky issue there. /CC @ozh in case he has an option.

Basically if yourls_escape is called before we've made a DB connection, it will fail, as it uses mysql_real_escape_string underneath. So in case where we have a cached options list we may never have connected, so mysql_real_escape_string will return false. We then are going to pass false to yourls_get_option, which itself will return false:

$safe_option_name = yourls_escape( $option_name );
$oldvalue = yourls_get_option( $safe_option_name );

We'll then return the yourls_add_option function, which will also call yourls_get_option. However, because a DB connection has been started at this point, yours_escape will return a proper string, which will then look up to an empty array (or whatever is in options), which is no === to false, so the add will quietly fail, and the DB wont be updated.

I suspect that the proper fix is either to use something other than mysql_real_escape_string, or to test for a false return a start a DB connection if we get one.

@ozh - thoughts? I can send a patch, but I'm not sure what the preferred route would be.

[ozh]

Not sure I understand the problem...

How can the plugin not be activated, since it's included via cache.php? Isn't there a confusion here letting the user think they have to activate it when it's not needed actually?

Also, regarding yourls_escape(), I don't think something has changed in this function for a very long time, so again I don't get why it wouldn't work with YOURLS 1.6 if it worked with earlier versions?

Please cast some lights on this :)

[ianbarber]

You're right about the plugin being activated, but to the user it doesn't seem so. This would block any plugin from being appearing to be activated though (as in the callback would be called, but the database would not be updated to mark the plugin as active).

Re the change in 1.6, I am guessing in previous version something would cause a DB connection to be started somewhere else before hitting this function in this place - it could be shuffling around of any, completely unrelated, bit of code. I guess the fundamental bug is:

A) Should yourls_escape ever return false for a valid string? At the moment it can, and that isn't handled in calling code.

[ozh]

yourls_escape() shouldn't return false if the string is valid, indeed. The thing is, I just don't understand why it would erroneously return false :-) The DB connection is created very early and I don't understand what part of your code triggers a yourls_escape() before $ydb is set and before the DB connection exists.

Here though, as I understand it, the problem is different, or at least there's also another problem. Since the plugin is always included, via cache.php, when the user tries to activate it, it gets included a second time, hence triggers notices about constants being redeclared (Notice: Constant APC_CACHE_LOG_INDEX already defined in ...), hence the activation fails/

I would do the following: in plugin.php, add some magic so that the plugin is always marked as 'activated'. Seems to work fine on my test install. I'm making a PR so you can see what I mean.

[ozh]

@huangzj @ianbarber Not sure if you're still following this -- YOURLS 1.7 has started dev with support for mysqli and PDO

Regarding that issue with yourls_escape() : could you update to 1.7 and replace functions-formatting.php/yourls_escape() (line 137ish) with the following:

function yourls_escape( $in ) {
    global $ydb;
    if( isset( $ydb->dbhz ) ) {
        return $ydb->escape( $in );
    } else {
        return addslashes( $in );
    }
}

I'd be very grateful for feedback on this