• 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Little bug fix for is_really_writable in common.php

#1
[eluser]Francois Gelinas[/eluser]
This is a small bug in codeigniter/common.php in function is_really_writable($file) that caused some request to the server to fail when there were many requests at the same time.
The problem occurred on my windows vista development station, with apache 2.2 and php 5 and might be caused by the behavior of chmod and unlink under windows.

When a request failed, a line was added to the log pointing to either the @chmod function of the @unlink function :
Code:
ERROR - 2009-04-21 16:24:02 --> Severity: Warning --> unlink(C:\web/system/cache/1c383cd30b7c298ab50293abdfecb7b18) function.unlink: Permission denied C:\web\system\codeigniter\Common.php 63

The problem is caused by the random file used in the function :
Code:
$file = rtrim($file, '/').'/'.md5(rand(1,100));
There is a maximum of 100 unique files used to test the function, chances are that the same file is used concurrently by 2 requests resulting in one of them to fail.
Replacing rand(1,100) by uniqid('',TRUE) fixed the problem on my machine.
Code:
$file = rtrim($file, '/').'/'.md5(uniqid('',TRUE));

I'm not sure if uniqid is the best thing to use here, but used with the more_entropy parameter seems to makes it very random.

#2
[eluser]Derek Jones[/eluser]
uniqid() has some performance implications on certain environments, and should be avoided in situations that might loop or get called frequently together. Basically on some servers use of that function triggers a sleep signal of around .02 seconds (instead of 1 millisecond like it's supposed to) so that it can be ensured that simultaneous requests don't get the same value for this call. May not sound like a lot, but 2 full seconds for 100 requests adds up. This might be an acceptable alternative though:

Code:
$file = rtrim($file, '/').'/'.md5(mt_rand(1,100).mt_rand(1,100));

Are you sure the permissions on this folder are correct, that it has not only read and write but also modify (or whatever it's referred to as in Windows these days)?

#3
[eluser]Francois Gelinas[/eluser]
Hi Derek, thanks for taking time to reply to my post.
The folders that are tested with the function is_really_writable() on my system are the log and cache folders, and CI is writing and updating files in those, so it's not a permissions problem.
I updated the file with your solution :
Code:
$file = rtrim($file, '/').'/'.md5(mt_rand(1,100).mt_rand(1,100));
and could not reproduce the problem yet, so it is random enough to fix the problem.
Just one question, is there a reason to use mt_rand(1,100) twice instead of mt_rand(1,10000) ?

#4
[eluser]Derek Jones[/eluser]
[quote author="Francois Gelinas" date="1240902765"]Just one question, is there a reason to use mt_rand(1,100) twice instead of mt_rand(1,10000) ?[/quote]

They're not strictly the same - since we're not padding the numbers, this leaves the possibility of a 3 or even 2 digit number. The main reason I chose two calls is that if the random number algorithm returns the same number twice in a row, the "damage" is mitigated. To result in the same problem you had before, it would require either four return values in a row to be identical, which is highly unlikely with the Mersenne Twister generator, and a pattern of 23, 1, 2, 31, etc. also has a lower probability of coming up than two numbers in a row from 1-10000.


Digg   Delicious   Reddit   Facebook   Twitter   StumbleUpon  


  Theme © 2014 iAndrew  
Powered By MyBB, © 2002-2021 MyBB Group.