httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rüdiger Plüm <r.pl...@gmx.de>
Subject Re: svn commit: r644746 - in /httpd/httpd/trunk: ./ docs/manual/mod/ include/ modules/session/ server/
Date Sat, 05 Apr 2008 15:58:13 GMT


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



Mime
View raw message