Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:56710 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 76806 invoked from network); 2 Dec 2011 04:35:11 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 2 Dec 2011 04:35:11 -0000 Authentication-Results: pb1.pair.com smtp.mail=dk@uw.no; spf=permerror; sender-id=unknown Authentication-Results: pb1.pair.com header.from=dk@uw.no; sender-id=unknown Received-SPF: error (pb1.pair.com: domain uw.no from 195.159.98.120 cause and error) X-PHP-List-Original-Sender: dk@uw.no X-Host-Fingerprint: 195.159.98.120 in.cluded.net Linux 2.4/2.6 Received: from [195.159.98.120] ([195.159.98.120:50688] helo=in.cluded.net) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id C0/32-57835-B7558DE4 for ; Thu, 01 Dec 2011 23:35:09 -0500 Received: (qmail 23699 invoked from network); 1 Dec 2011 06:23:22 -0000 Received: from 78.225.251.212.customer.cdi.no (HELO ?10.0.0.100?) (daniel@212.251.225.78) by in.cluded.net with ESMTPA; 1 Dec 2011 06:23:22 -0000 Message-ID: <4ED85560.6040101@uw.no> Date: Fri, 02 Dec 2011 04:34:40 +0000 User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060307 SeaMonkey/1.5a MIME-Version: 1.0 To: Yasuo Ohgaki CC: Rasmus Lerdorf , internals@lists.php.net References: <4EBDC283.3040700@yahoo.co.jp> <4ED727FB.7030001@uw.no> <4ED76013.50601@lerdorf.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [PHP-DEV] Strict session? From: dk@uw.no ("Daniel K.") Yasuo Ohgaki wrote: > 2011/12/2 Yasuo Ohgaki : >> 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.