Project

General

Profile

Actions

Bug #104155

closed

inconsistence of database definition for field sys_refindex.ref_uid (possible overflow)

Added by Bernd Wilke 11 days ago. Updated 10 days ago.

Status:
Rejected
Priority:
Should have
Assignee:
-
Category:
Database API (Doctrine DBAL)
Target version:
Start date:
2024-06-19
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
12
PHP Version:
8.1
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

is this a possible point of failuer in TYPO3 core?
while having another failure thrown with lolli/dbdoctor (https://typo3.slack.com/archives/C025BQLFA/p1718718362856409) I think I found an inconsistence in core database definitions:
It's about definitions and usage of `sys_refindex.ref_uid while I understand that uids (unsigned int) are stored I also found some records with negative numbers for tables fe_users and sys_languages`, which might be valid as the negative values are used for 'any language', 'any login', 'no login'

as the field sys_refindex.ref_uid is defines as (signed) int it might fail if there is stored an uid from the upper half range of unsigned int (> 2147483647 )

is it intended that negative numbers are stored? (as the field is defined as type int) => what will happen if a uid like 3147483647 has to be stored?

a possible solution might be to use `bigint` for this field (or a special handling if big uids occur)

Actions #1

Updated by Christian Kuhn 10 days ago ยท Edited

  • Status changed from New to Rejected

sys_refindex.rec_uid is signed. v13 recently added this comment:

    # @todo: ref_uid is still signed since refindex tends to write -2 for fe_group "all" relations.
    #        EidRequestTest.php PlainScenario.yaml triggers this and fails with mariadb.
    #        This is about "not real db relations" in refindex and needs to be sorted out
    #        including some dedicated tests.
    ref_uid int DEFAULT 0 NOT NULL,

So yeah, it is signed due to sys_language_uid and fe_groups select can be -1 or -2. I do not like this situation and I think it should be sorted out in some way since those are "not real" relations being stored in refindex. Note sys_language_uid=-1 needs to fall for many further reasons anyway. I'm still undecided on how to deal with -1 and -2 in sys_refindex, but core currently does not actively use this information, which is why there was no true pressure to clean this up, yet.

So, at the moment, we have this situation: table 'uid' columns are unsigned int, so 0 - 2^32-1 (0 - 4,x billion), while rec_uid is -2^31 - 2^31-1 (-2,x billion - 2,x billion).

So yes, this is a potential conflict. However, I do not think we should resolve this by making rec_uid a bigint:

  • First, 2,x billion is so high that very most likely no instance will hint the 31 bit barrier. I think very big instances that did not tamper with auto increment offsets are somewhere around a couple of million, not billion. I'd estimate even big instances still have three orders of magnitude "space".
  • Instances that did tamper with auto increments to then hit the 2 or 4 billion barrier should be considered buggy, the core does not need to support this.
  • Instances that did tamper with auto increments and decide not to be fixed could increase uid fields and/or sys_refindex.ref_uid and sys_refindex.recuid themselfs by adding according entries in some ext_tables.sql. This should be considered an instance specific technical dept that should be fixed.
  • int has advantages by index length, bigint can be slower. int should be preferred if possible, and it is possible for uid fields in core, so we should stick with it.
  • signed vs. unsigned: postgres has no unsigned int, they are always signed. positive int-max with postgres 2^31-1. The "conflict" thus only exists with DB's that support unsigned.

-> I think we should continue to work on refindex and "virtual" relations like -1 and similar, they are a bad idea for many reasons. When they are gone, the "conflict" between table.uid and sys_refindex.ref_uid is gone by making ref_uid unsigned. The raised issue is more a theoretical / consistency thing and needs no direct action. We can easily keep the current situation until -1 and friends are sorted out.

I hope it's ok to set the issue to rejected for now.

Actions

Also available in: Atom PDF