Fixed sql binding not being unique on Windows

[eXpl0it3r]

uniqid() on Windows has a low chance of being unique, which causes the binding ids to clash and totally destroy the SQL queries. Allowing uniqid() to generate more entropy makes it unique again, but introduces a dot, which then fails with the associative key lookup.

References

• PHP documentation
• Similar fix was applied for example in Typo3
• Informative SO answer

[lukasbestle]

Thanks for the insights!

I think the best solution would be to use an internal counter and generate binding IDs like binding1 etc. Then they will always be guaranteed to be unique.

I currently can't say whether we will make this change in Kirby 2 though. Kirby 3 won't have database classes in its core (there are much more stable alternatives out there) and we need to decide which kind of bugs we will fix in Kirby 2 after Kirby 3 is out.

[eXpl0it3r]

I understand that you have to evaluate what can get done for Kirby 2. Now, unfortunately, the database feature is completely unusable on Windows and potentially broken on other systems.

I could also provide a fix with a unique counter, but will only invest that time, if you're willing to press the merge button. 😉

[lukasbestle]

We have discussed this in the team and decided that we will have database classes in Kirby 3. In the upgraded v3 implementation, we have switched to random_bytes() to generate the binding names. The entropy should be enough to ensure unique binding names on all platforms.

I'm keeping this issue open anyway in case we will fix this in v2 as well after the v3 launch.

[eXpl0it3r]

Does that mean, that this bug will not be fixed in Kirby 2? The database feature becomes thus unusable on Windows.

[bastianallgeier]

@eXpl0it3r yes, unfortunately this is what it means. We have an updated DB class in v3.

[eXpl0it3r]

So why update the toolkit to PHP 7.4 but not include this already provided fix?

[bastianallgeier]

Don't get me wrong. This really seems like an easy fix. But PRs always tend to take much longer to test and include in a release than expected. Does it really work as expected (in this case on unix and win)? Does it have any side effects. Etc. We don't have a enough tests for v2 that would help us avoid regressions and that makes every bug fix risky.

We decided against fixing bugs in general in v2 unless necessary to keep Kirby secure on public servers.

[eXpl0it3r]

I do understand that resources are finite, that v3 is where the future is at and that not all bugs are created equally.

Sadly this is a blocker and it's rather annoying having to maintain my own version for a one-liner fix that should be fairly easy understood, given you know the inner workings of the code.