Bug #59701

uniqid() not returning unique values

Added by Tymoteusz Motylewski almost 6 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Should have
Category:
-
Target version:
-
Start date:
2014-06-20
Due date:
% Done:

100%

TYPO3 Version:
6.2
PHP Version:
Tags:
Complexity:
Is Regression:
No
Sprint Focus:

Description

uniqid() generates values based on current time,
subsequent calls may return the same value on a fast machine.

On Windows it's even worse, as uniqid()
has single-second-resolution out of the box.

Right now it is used in many places in the core, also for creating temporary identifiers for newly created records (in the datahanlder)

The solution is to add a second parameter to all calls (which adds more entropy).
see http://php.net/manual/en/function.uniqid.php

uniqid("prefix") => uniqid("prefix", TRUE)

With an empty prefix, the returned string will be 13 characters long. If more_entropy is TRUE, it will be 23 characters. So we need to test whether having longer id doesn;t break anything.


Related issues

Related to TYPO3 Core - Bug #59529: Make Functional tests work on fast machines (especially on Windows) Closed 2014-06-12
Related to TYPO3 Core - Bug #58602: Datepicker issue on multiple datetime fields in BE Closed 2014-05-07
Related to TYPO3 Core - Bug #59055: Import from .t3d failed on Windows 7 with a lot of messages Closed
Related to TYPO3 Core - Bug #58768: Import from .t3d failed on Windows 7 with a lot of messages (6.1 => 6.2) Rejected 2014-05-13
Related to TYPO3 Core - Bug #62438: Fix JS error after #30948 Closed 2014-10-24
Related to TYPO3 Core - Bug #63943: PHPUnit-Tests fail on Windows Closed 2014-12-16
Related to TYPO3 Core - Task #69050: Supply a Utility method to create unique ids in the core Closed 2015-08-14
Related to TYPO3 Core - Bug #91553: Risk of non-unique field in DatePickerViewHelper Under Review 2020-06-02

Associated revisions

Revision fa817a7e (diff)
Added by Tymoteusz Motylewski over 5 years ago

[BUGFIX] Add more entropy to uniqid

uniqid() generates values based on current time,
subsequent calls may return the same value on a fast machine.

On Windows it's even worse, as uniqid()
has single-second-resolution out of the box.

Right now it is used in many places in the core,
also for creating temporary identifiers
for newly created records (in the datahandler).

The solution is to add a second parameter to
all calls (which adds more entropy).
see http://php.net/manual/en/function.uniqid.php

To make code consistent, this change adds the
second parameter to all occurences of uniqid.

Resolves: #59701
Resolves: #58602
Resolves: #59055
Releases: master, 6.2
Change-Id: Id791556d45b4289d75411ff19ae050144fbfe51b
Reviewed-on: http://review.typo3.org/30948
Reviewed-by: Anja Leichsenring <>
Tested-by: Anja Leichsenring <>
Reviewed-by: Wouter Wolters <>
Tested-by: Wouter Wolters <>
Reviewed-by: Stefan Froemken <>
Tested-by: Stefan Froemken <>
Reviewed-by: Markus Klein <>
Tested-by: Markus Klein <>

Revision 5df3d530 (diff)
Added by Tymoteusz Motylewski over 5 years ago

[BUGFIX] Add more entropy to uniqid

uniqid() generates values based on current time,
subsequent calls may return the same value on a fast machine.

On Windows it's even worse, as uniqid()
has single-second-resolution out of the box.

Right now it is used in many places in the core,
also for creating temporary identifiers
for newly created records (in the datahandler).

The solution is to add a second parameter to
all calls (which adds more entropy).
see http://php.net/manual/en/function.uniqid.php

To make code consistent, this change adds the
second parameter to all occurences of uniqid.

Resolves: #59701
Resolves: #58602
Resolves: #59055
Releases: master, 6.2
Change-Id: Id791556d45b4289d75411ff19ae050144fbfe51b
Reviewed-on: http://review.typo3.org/33328
Reviewed-by: Markus Klein <>
Tested-by: Markus Klein <>

History

#1 Updated by Tymoteusz Motylewski almost 6 years ago

another thing to keep in mind while testing is that uniqid("", TRUE) returns id which contains a dot, e.g. "53a4364e8598c4.29100310"
Implementation:

if (more_entropy) {
        spprintf(&uniqid, 0, "%s%08x%05x%.8F", prefix, sec, usec, php_combined_lcg(TSRMLS_C) * 10);
    } else {
        spprintf(&uniqid, 0, "%s%08x%05x", prefix, sec, usec);
    }

https://github.com/php/php-src/blob/af6c11c5f060870d052a2b765dc634d9e47d0f18/ext/standard/uniqid.c

#2 Updated by Gerrit Code Review almost 6 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/30948

#3 Updated by Gerrit Code Review almost 6 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/30948

#4 Updated by Gerrit Code Review almost 6 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/30948

#5 Updated by Gerrit Code Review almost 6 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/30948

#6 Updated by Gerrit Code Review almost 6 years ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/30948

#7 Updated by Gerrit Code Review over 5 years ago

Patch set 7 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/30948

#8 Updated by Gerrit Code Review over 5 years ago

Patch set 8 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/30948

#9 Updated by Gerrit Code Review over 5 years ago

Patch set 9 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/30948

#10 Updated by Gerrit Code Review over 5 years ago

Patch set 10 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/30948

#11 Updated by Gerrit Code Review over 5 years ago

Patch set 11 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/30948

#12 Updated by Tymoteusz Motylewski over 5 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#13 Updated by Gerrit Code Review over 5 years ago

  • Status changed from Resolved to Under Review

Patch set 1 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/33328

#14 Updated by Gerrit Code Review over 5 years ago

Patch set 2 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/33328

#15 Updated by Tymoteusz Motylewski over 5 years ago

  • Status changed from Under Review to Resolved

#16 Updated by Roman Eberle about 5 years ago

i think a better solution would be to create something like

...\GeneralUtility\UniqueId()

which unifies the generation of unique ids, and replace all calls to uniqid() with calls to that function.
this might be done with a smart shell-script (grep/sed), should fix pretty much all uniqid()-related errors, and allows easy modification for possible future changes to unique-id-generation.

note that I encountered typo3-exceptions related to PATTERN_ENTRYIDENTIFIER in sysext/core/Classes/Cache/Frontend/FrontendInterface.php with TYPO3 6.2.12, these don't seem to be covered by the above commits/patches.

#17 Updated by Stephan Großberndt almost 5 years ago

Hello Roman,

I just created a ticket for that.
Any additional thoughts?

https://forge.typo3.org/issues/69050

#18 Updated by Benni Mack over 1 year ago

  • Status changed from Resolved to Closed

#19 Updated by Stephan Großberndt 4 days ago

  • Related to Bug #91553: Risk of non-unique field in DatePickerViewHelper added

Also available in: Atom PDF