Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Message-ID: <47F7A195.3050801@gmx.de>
Date: Sat, 05 Apr 2008 17:58:13 +0200
From: =?ISO-8859-1?Q?R=FCdiger_Pl=FCm?= <r.pluem@gmx.de>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US;
 rv:1.8.1.13) Gecko/20080313 SeaMonkey/1.1.9
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: Re: svn commit: r644746 - in /httpd/httpd/trunk: ./ docs/manual/mod/
 include/ modules/session/ server/
References: <20080404155822.A0C211A9832@eris.apache.org>
 <47F788DF.8000702@apache.org> <47F79A19.10609@sharp.fm>
In-Reply-To: <47F79A19.10609@sharp.fm>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 8bit


On 04/05/2008 05:26 PM, Graham Leggett wrote:
> Ruediger Pluem wrote:
> 
>>> +AP_DECLARE(int) ap_unescape_all(char *url);
>>> +
>>> +/**
>>
>> Doesn't this require a minor bump?
> 
> I think it does do, yes. Fixed.
> 
>>> +AP_DECLARE(char *) ap_escape_path_segment_b(char *c, const char *s);
>>
>> This is not a very descriptive name. Shouldn't it be better 
>> ap_escape_path_segment_buffer?
> 
> It was originally buffer, then I changed it because it was too long. 
> You're right though, will change.

Just saw it. I guess you missed to fix the name in the modules that call it.

> 
>>> +AP_DECLARE(void) ap_session_get(request_rec * r, session_rec * z, 
>>> const char *key, const char **value)
>>> +{
>>> +    if (!z) {
>>> +        ap_session_load(r, &z);
>>
>> Not checking the return value can lead to segfaults as z can be invalid.
> 
> The real check is to ensure that z is set, the return value can be 
> successful but z can still be NULL (which will happen if no session is
> configured), so checking the return value won't help.

Ok, but then we should ensure that z will be at least NULL for proper checking.
AFAICT we do not set *z to NULL in ap_session_load in the case of an error.
So it z may point to an arbitrary location in the memory.

Regards

R�diger