joshp23 / YOURLS-Expiry

YOURLS plugin to define conditions under which links will expire - time and click limited links
GNU General Public License v3.0
35 stars 13 forks source link

Expiry not working - Binary type not compatible with aura/sql fetchOne method #36

Closed Omniru closed 3 years ago

Omniru commented 3 years ago

Binary type not compatible with aura/sql fetchOne method

https://github.com/joshp23/YOURLS-Expiry/blob/5bf6c7234376943b0f1522d8ea2eafaec4cce0a3/expiry/plugin.php#L1328

https://github.com/joshp23/YOURLS-Expiry/blob/5bf6c7234376943b0f1522d8ea2eafaec4cce0a3/expiry/plugin.php#L790

In the latest version 2.4.0 I had issues with the create statement creating the field as a binary type instead of a varchar(200) like is shown in the expiry.sql code. After changing that type to varchar yourl's aura/sql implementation of fetchOne is able to find the record in the database and successfully delete them as expected.

`yourls_add_action( 'activated_expiry/plugin.php', function () {

global $ydb; // Create the expiry table $table = YOURLS_DB_PREFIX . 'expiry'; $table_expiry = "CREATE TABLE IF NOT EXISTS ".$table." ("; $table_expiry .= "keyword varchar(200) NOT NULL, "; $table_expiry .= "type varchar(5) NOT NULL, "; $table_expiry .= "click varchar(5), "; $table_expiry .= "timestamp varchar(20), "; $table_expiry .= "shelflife varchar(20), "; $table_expiry .= "postexpire varchar(200), "; $table_expiry .= "PRIMARY KEY (keyword) "; $table_expiry .= ") ENGINE=InnoDB DEFAULT CHARSET=latin1;";

$tables = $ydb->fetchAffected($table_expiry); });`

joshp23 commented 3 years ago

i pushed this. Thanks for the fix, maybe consider looking into learning about pull requests?

Omniru commented 3 years ago

I am curious if you had it set to a binary type to address issues with using a character set that's a-z A-Z0-6, since the behavior I now see from REPLACE INTO given a latin1 latin1_sweedish_ci collation in mariadb, and likely other databases, will cause it to overwrite different casing and cause links not to expire.

On Thu, Jun 17, 2021 at 4:37 PM Josh Panter @.***> wrote:

i pushed this. Thanks for the fix, maybe consider looking into learning about pull requests?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/joshp23/YOURLS-Expiry/issues/36#issuecomment-863606345, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACR7PUPILDYNAXKUYWFZS2TTTJ2KXANCNFSM46XYU73Q .

Omniru commented 3 years ago

I ended up formatting the table like this to make it respect casing, modeled after yourls_url table. I am still testing it though. --I'm also unsure if bigint is an issue for other databases or not as I'm not sure what's normally supported.

// Create tables for this plugin when activated
yourls_add_action('activated_expiry/plugin.php', function () {

    global $ydb;
    // Create the expiry table
    $table        = YOURLS_DB_PREFIX . 'expiry';
    $table_expiry = "CREATE TABLE IF NOT EXISTS `" . $table . "` (";
    $table_expiry .= "keyword varchar(100) COLLATE utf8mb4_bin NOT NULL DEFAULT '', ";
    $table_expiry .= "type varchar(5) NOT NULL, ";
    $table_expiry .= "click varchar(5), ";
    $table_expiry .= "timestamp bigint(20), ";
    $table_expiry .= "shelflife int(11), ";
    $table_expiry .= "postexpire varchar(200), ";
    $table_expiry .= "PRIMARY KEY (keyword) ";
    $table_expiry .= ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin;";

    $tables = $ydb->fetchAffected($table_expiry);
});