php / php-src

The PHP Interpreter
https://www.php.net
Other
38.17k stars 7.74k forks source link

dba_insert() for ini files can result in duplicate keys #10891

Open nielsdos opened 1 year ago

nielsdos commented 1 year ago

Description

The following code:

<?php

@unlink("test.ini");
touch("test.ini");
$db = dba_open("test.ini", "wl", "inifile");
var_dump($db);

var_dump(dba_fetch("nonexisting", $db));
var_dump(dba_insert("nonexisting", "newvalue", $db));
var_dump(dba_insert("nonexisting", "newvalue", $db));

dba_close($db);

Resulted in this output:

resource(7) of type (dba)
bool(false)
bool(true)
bool(true)

But I expected this output instead:

resource(7) of type (dba)
bool(false)
bool(true)
bool(false)

And additionally: not an extra same-key entry in my ini file. Because according to the docs at https://www.php.net/manual/en/function.dba-insert.php:

If this key already exist in the database, this function will fail.

PHP Version

PHP 8.1+

Operating System

Linux 6.2.x

marcosmarcolin commented 1 year ago

Hello @nielsdos

I don't know if this is already fixed (I believe so).

But I tried it out of curiosity and came up with this: https://github.com/marcosmarcolin/php-src/commit/0af0d65dac015130e36bed64055ada5a944ea5a8

I believe it fixed it, but I'm not sure it's ideal.

nielsdos commented 1 year ago

Thanks for your interest in fixing this issue. I don't know if the issue is exclusive to inifile or if other dba handlers have the same issue. I'd have to test this and we have to know this first before trying to fix this. If the issue only happens on inifile, then it should be fixed in that handler.

Girgias commented 1 year ago

Been testing this on a bunch of DBA drivers (db4, gdbm, qdbm, flatfile, lmdb, tcadb) where they all behave as expected.

Still trying to get the test to work for CDB but it looks like it's an INIFILE only bug.

This also affects CDB

Girgias commented 1 year ago

Okay, I need to have a longer think about this.

Because trying to fix this seems to go against the behaviour that is tests in the regression test for Bug #70825 (Cannot fetch multiple values with group in ini file).

The issue seems to be that for handlers that support key grouping dba_insert() behaves more like a fictional dba_append().

So not sure how to approach this, tests are written and available here: https://github.com/Girgias/php-src/tree/dba-gh10891