SHA256 version 2.0 alpha 3 - updated

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

Your point being?
User avatar
Ambush Commander
DevNet Master
Posts: 3698
Joined: Mon Oct 25, 2004 9:29 pm
Location: New Jersey, US

Post by Ambush Commander »

Feyd is talking about explicit-ness. It's not immediately obvious from === that type is being checked too.
User avatar
Oren
DevNet Resident
Posts: 1640
Joined: Fri Apr 07, 2006 5:13 am
Location: Israel

Post by Oren »

If they are not of the same type, === will return false. This is exactly the behavior feyd wanted, so I don't see why the extra is_bool() is needed.
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

To make sure people understand what's happening. The time difference is extremely negligible.
User avatar
Oren
DevNet Resident
Posts: 1640
Joined: Fri Apr 07, 2006 5:13 am
Location: Israel

Post by Oren »

If someone doesn't know what === does, they shouldn't be reading this code anyways.
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

The code isn't meant to be read by many. This nitpick is pointless.

Let's move on, shall we?
User avatar
Oren
DevNet Resident
Posts: 1640
Joined: Fri Apr 07, 2006 5:13 am
Location: Israel

Post by Oren »

Ok, ok... it's just that you were the last person I expected to write a code like:

Code: Select all

$this->noExt = is_bool($noExt) && $noExt === true;
:P
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

The code isn't about speed so much as it is in readability (as much as possible anyways) for users who are curious enough to look. That's why it's commented even more than the original.

Unfortunately, the workhorse function(s) cannot easily be rewritten for easier "newb" reading, although I did my best to make it easier to follow what's happening.
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

Yeah personally I think this is_bool() thing confuses things greatly, even once I read it and understand what PHP is doing I'm still left wondering why it has been written that way. But yeah this is very nitpicky so I'm not going to say anymore.
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Post by Benjamin »

I believe that in the Urlinput.class.php for PHP 5, in the following code, $filename should be $url.

Code: Select all

if (!is_scalar($url))
		{
			trigger_error('UrlInput(' . var_export($filename, true) .
				'): cannot be opened.',
				E_USER_ERROR);
		}
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

Oops. You are indeed correct. That's what I get for repurposing the entire thing from FileInput. :) .. not to mention testing everything I'm checking for. ;)
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Post by Benjamin »

Yeah stuff like that is easy to miss. I was looking through your code to see if I could pick up any new tricks. What stuck out the most is your use of is_scalar. I can't imagine any decent programmer passing an array instead of a string, but stranger things have happened I'm sure. Nevertheless, now I know how to make sure something isn't an object, array or resource :)
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

Yeah, is_scalar() is a nice little function not often used.
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

Hi Feyd,

Been using the code quietly since the very first release ;). I have almost a dozen applications in PHP4 depending on it. In a month or two it may also be bundled in another public library for PHP4 (already have a PHP5 specific version) for OpenID 2.0 which recommends SHA256 HMAC signatures. You can guess who'll be responsible for the bumped up security needed ;).

Kudos...again... This is a really useful class to have around.
KalleL
Forum Newbie
Posts: 12
Joined: Mon Apr 30, 2007 8:52 am

Post by KalleL »

Hi,

I'm having strange issue with latest (?) release 2.0 alpha v. 3. I have PHP version 5.0.5 according phpinfo(); and cPanel. Still i'm getting parse error if i'm using PHP 5 version of this SHA256 files. When using PHP 4 files, everything ok.

It isn't that big problem as it's working with PHP 4 files, but would be nice to know whether it is a problem with the script or web server.


Greetings,
Kalle
Post Reply