Re: [PHP-DEV] Strict session?

Yasuo Ohgaki wrote:
> 2011/12/2 Yasuo Ohgaki <yohgaki <at> ohgaki.net>:
>> I think Daniel mean there are extra spaces for indent.
>> I'll fix it.

That's exactly it, however the updated patch still has problems.

Search for a + followed by only tabs or spaces. Empty lines should be
just that, empty.

>> Since Daniel mentioned that he cannot disable strict session,

I did no such thing. from where did you get that idea?

I mentioned that I had not tested the patch at all, just read it.

> I also updated tests that are exploiting adoptive sessions.
> 
> https://gist.github.com/1379668

I see you've addressed a few of my comments, but not all.

Specifically, excessive whitespace remains. Both on otherwise empty 
lines, and on lines indented with a tab and a space, which just looks 
sloppy. See e.g. PS_VALIDATE_SID_FUNC(mm) as an example showing both 
defects. You've only fixed it in ext/session/session.c, do the same 
to the rest of the patch.

These comments should either be fixed to match the code, or deleted.

+		/* valid characters are a..z,A..Z,0..9 */
+		if (!((c >= 'a' && c <= 'z')
+			   || (c >= 'A' && c <= 'Z')
+			   || (c >= '0' && c <= '9')
+			   || c == ','
+			   || c == '-')) {

ps_user_valid_key returns SUCCESS/FAILURE.
ps_mm_valid_key returns 1/0 as does the exsting ps_files_valid_key.

I am in serious doubt as to whether the additonal restrictions on valid
characters in session ids are appropriate, and I fear that some poor sod 
may be in for a nasty surpris because of this.

Remember, this is not just about the return value of hash functions, as 
this is used to validate session_ids set with session_id() as well.

Daniel K.

--

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php