php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #65626 uniqid(null, false) should guarantee unique identifier
Submitted: 2013-09-05 22:46 UTC Modified: 2017-11-21 11:42 UTC
Votes:2
Avg. Score:5.0 ± 0.0
Reproduced:2 of 2 (100.0%)
Same Version:0 (0.0%)
Same OS:2 (100.0%)
From: cmbecker69 at gmx dot de Assigned: cmb (profile)
Status: Closed Package: Unknown/Other Function
PHP Version: 5.4.19 OS: WIN32
Private report: No CVE-ID: None
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If this is not your bug, you can add a comment by following this link.
If this is your bug, but you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: cmbecker69 at gmx dot de
New email:
PHP Version: OS:

 

 [2013-09-05 22:46 UTC] cmbecker69 at gmx dot de
Description:
------------
From looking at the sources[1], it occurs to me that 
uniqid(null, false) doesn't *guarantee* to return a 
unique identifier on WIN32, because usleep(3) is skipped.

As usleep() is implemented as PHP userland function since
PHP 5.0.0[2], it seems to be reasonable and possible to add 
the equivalent to usleep(3) there.

As far as I can tell, this won't introduce any BC issues, so
the change might be done for PHP 5.4.x.

Please note that this is related to #37106[3], but helly's 
comment doesn't seem to fit to the current sources.

[1] <http://lxr.php.net/xref/PHP_5_4/ext/standard/uniqid.c#61>
[2] <http://lxr.php.net/xref/PHP_5_4/win32/time.c#96>
[3] <https://bugs.php.net/bug.php?id=37106>

Test script:
---------------
var_dump(uniqid(null, false) != uniqid(null, false));

Expected result:
----------------
bool(true)

Actual result:
--------------
bool(false) // at least sometimes on a fast machine ;)

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2014-02-08 20:30 UTC] ab@php.net
-Status: Open +Status: Not a bug
 [2014-02-08 20:30 UTC] ab@php.net
I think helly's comment is pretty much matching. Also, an integral part of that function relies on the microtime API, but those implementation is platform dependent. For instance, on win8 a snippet "while(uniqid(null, false) != uniqid(null, false));" loops forever with 5.4 even if it doesn't have to. Otherwise you can use usleep() in userspace to ensure some better result.
 [2014-02-08 20:51 UTC] yohgaki@php.net
It seems uniqid() has protection under UNIXes.

#if HAVE_USLEEP && !defined(PHP_WIN32)
    if (!more_entropy) {
#if defined(__CYGWIN__)
        php_error_docref(NULL TSRMLS_CC, E_WARNING, "You must use 'more entropy' under CYGWIN");
        RETURN_FALSE;
#else
        usleep(1);
#endif
    }
#endif

Why is this disabled for windows?
 [2014-02-08 21:09 UTC] yohgaki@php.net
-Status: Not a bug +Status: Re-Opened -Type: Feature/Change Request +Type: Bug
 [2014-02-08 21:09 UTC] yohgaki@php.net
#if HAVE_USLEEP && !defined(PHP_WIN32)

!defined(PHP_WIN32) seems legacy check to me as PHP's usleep() only checks HAVE_USLEEP.

#if defined(__CYGWIN__)

Why Cygwin requires more entropy? I guess the #if check is for assuring uniqueness of uniqid(). Is this needed still?

I think we need Windows experts.
 [2014-02-08 22:03 UTC] ab@php.net
-Status: Re-Opened +Status: Feedback
 [2014-02-08 22:03 UTC] ab@php.net
Hi,

I'm not sure why you reopen this ticket :) If you read the helly's comment in bug #37106, it's pretty matching. uniqid() isn't in any way expected to be predictable on different platforms.

Btw. never tried to compile it under cygwin. Also see the history, that part was introduced in 2002. Were cygwin builds ever been seriously supported?

So where you see a bug here? Nevertheless, the uniqid function is documented as not being secure. usleep() can be used from the userspace and the value is highly variable depending on the platform and hardware.

Thanks.
 [2014-02-09 12:16 UTC] cmbecker69 at gmx dot de
helly wrote:

| To ensure this we use usleep() internally to either force a 
| thread (windows) or process (*nix) switch.

From what I can tell, usleep() is not called on Windows.[1]

ab wrote:

| For instance, on win8 a snippet "while(uniqid(null, false) 
| != uniqid(null, false));" loops forever with 5.4 even if 
| it doesn't have to.

However, the documentation[2] states:

| Gets a prefixed unique identifier based on the current time
| in microseconds.

IMHO, a unique identifier should be unique.

I have not changed the status, because I had not re-opened the
ticket nor had I reported it as bug, but rather as a feature
request.

[1] <http://lxr.php.net/xref/PHP_TRUNK/ext/standard/uniqid.c#61>
[2] <http://php.net/manual/en/function.uniqid.php>
 [2014-02-09 12:16 UTC] cmbecker69 at gmx dot de
helly wrote:

| To ensure this we use usleep() internally to either force a 
| thread (windows) or process (*nix) switch.

From what I can tell, usleep() is not called on Windows.[1]

ab wrote:

| For instance, on win8 a snippet "while(uniqid(null, false) 
| != uniqid(null, false));" loops forever with 5.4 even if 
| it doesn't have to.

However, the documentation[2] states:

| Gets a prefixed unique identifier based on the current time
| in microseconds.

IMHO, a unique identifier should be unique.

I have not changed the status, because I had not re-opened the
ticket nor had I reported it as bug, but rather as a feature
request.

[1] <http://lxr.php.net/xref/PHP_TRUNK/ext/standard/uniqid.c#61>
[2] <http://php.net/manual/en/function.uniqid.php>
 [2014-02-10 10:56 UTC] ab@php.net
@cmbecker69, with reopening it was addressed to Yasuo. Uniqueness of that identifier is applicable in the microseconds range only. That scope is also documented, so the behavior is legit. That's the main idea in the helly's post, i think. And his post relates back to 2006 while the relevant code to 2002. For the reasons not to do it on native windows, see below.

@yasuo, I've tested your patch on the native windows builds on different VM, it delivers so far without having to use usleep() in the user space. @cmbecker69, could you please test this build http://windows.php.net/downloads/snaps/ostc/65626/ with Yasuo's patch?

Still I wouldn't push for this change, testing in VM is something else as the real hardware. Unfortunately I've VMs only right now. Besides that, working on the microtime() bugs has shown that many different constellations of hardware/platform cause various disservices. So fixing it for several constellations contains a high possibility to break it for others. Also in the light of how win8 works now having much better time accuracy, it's probably not future oriented. A timeout is still something delaying the script execution, even for 1us, that's what a PHP developer can decide for the concrete script.

So i really think there are more factors against changing this part, except cygwin maybe. The cygwin part is still to be tested by someone who has the build env.

Thanks.
 [2014-02-10 14:29 UTC] cmbecker69 at gmx dot de
-Status: Feedback +Status: Open
 [2014-02-10 14:29 UTC] cmbecker69 at gmx dot de
I have tested the build with Yasuo's patch on Windows 7
Home Premium SP 1, and it works fine, i.e. I was not able
to produce identical uniqid()s.  I couldn't test it on an
old XP Home SP 3 machine, though, as the build seems to be 
a 64bit build.

> A timeout is still something delaying the script execution,
> even for 1us, that's what a PHP developer can decide for 
> the concrete script.

Yes, of course.  However, a portable script would have to 
add the call to usleep(), even if it's unnecessary for *nix 
systems and actually causes a delay of 2 us there.

And actually, I don't see a real issue with using usleep() in
uniqid(), as usleep() is used by PHP's usleep()[1], so there
would be issues as well.  Shifting the burden of calling usleep()
to userland won't solve these issues.

Regarding Cygwin: I did some builds of PHP 5.4.? and PHP 5.5.? 
more than half a year ago.  However, not everything was usable
(e.g. Opcache had to be disabled; otherwise ./configure failed).
And the test suite produced a bunch of failures; I tried to 
submit the resulting report (nearly 1 MB) to the QA list, but
somehow it never made it through. 

FWIW: I didn't manage to build PHP on Cygwin64.

[1] <http://lxr.php.net/xref/PHP_5_5/ext/standard/basic_functions.c#4461>
 [2014-02-10 16:20 UTC] ab@php.net
-Status: Open +Status: Feedback
 [2014-02-10 16:20 UTC] ab@php.net
@cmbecker69, thanks for testing. It won't work on XP as it's a master+patch build with vc11.

The issue isn't directly with usleep() but with the subsequent gettimeofday(). What I meant earlier about microtime, that even after hardcoded usleep(1) gettimeofday still could deliver some inaccurate result, which would mean same old behaviour and the waste of that 1us. And that is something we can't do much about, because it depends on hardware and platform. Well, except of using the newer APIs. So that can't be the same as on linux by best will, please read bug #64633 for more info. A portable script will have to check a lot of things anyway.

I'm really not sure whether someone works with cygwin that way. Native builds are of much better quality, and if one needs the unix style - so there's a VM :)

Thanks.
 [2014-02-10 17:48 UTC] cmbecker69 at gmx dot de
-Status: Feedback +Status: Open
 [2014-02-10 17:48 UTC] cmbecker69 at gmx dot de
Thanks, Anatol, for the explanation and the link to the other
ticket.  Finally, I understand the problem, and that it can't
be cleanly fixed for older Windows systems.

Regarding the Cygwin issue see bug #27182.  It seems the patch 
provided by sagawa works fine.  However, I agree that Cygwin
support is of minor importance nowadays.
 [2017-11-21 01:06 UTC] 601115211 at qq dot com
<?php
$str = 'HTTP_TOKEN';
echo "PHP_VERSION:",PHP_VERSION,PHP_EOL;
echo ltrim($str, 'HTTP_');

Expected result:
----------------
TOKEN

Actual result:
--------------
OKEN
 [2017-11-21 11:42 UTC] cmb@php.net
-Status: Open +Status: Closed -Type: Bug +Type: Feature/Change Request -Assigned To: +Assigned To: cmb
 [2017-11-21 11:42 UTC] cmb@php.net
Since the documentation clearly states that uniqid() is not
guaranteed to return unique values, this is not a bug, but rather
a feature request.  Since there is already an RFC addressing this
issue[1], I'm closing this ticket.

@601115211: Please do not file bug reports to existing but
unrelated tickets.  File a new bug report instead.  However, in
this case it is not necessary, because the behavior of ltrim() is
expected, because the second parameter is supposed to be a
character *mask*.  Try ltrim($str, 'HTP_') to understand the
behavior.

[1] <https://wiki.php.net/rfc/uniqid>
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Wed Oct 23 03:01:29 2019 UTC