Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Message-ID: <459A4460.6050604@apache.org>
Date: Tue, 02 Jan 2007 12:39:12 +0100
From: Ruediger Pluem <rpluem@apache.org>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.13) Gecko/20060417
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: Re: archive.apache.org issues
References: <4591C260.2090202@apache.org>
 <4592ED0E.9000509@apache.org>	 <5c902b9e0612280804p21f5bb98w3a8679f2964bbed7@mail.gmail.com>
	 <459455E7.4020809@apache.org>
	 <Pine.LNX.4.64.0612290427010.7833@west.cs.uu.nl>
	 <5c902b9e0612282033o6eeebb48t5313b924f4f6cdbe@mail.gmail.com>
	 <4594A4F5.8050404@apache.org>
 <4595ADB0.1040702@apache.org>	 <4596495B.2000700@apache.org>
 <5c902b9e0612300929u3881f3f7q401a0a8a139cd3c7@mail.gmail.com>
 <45980CC9.1000004@rowe-clan.net>
In-Reply-To: <45980CC9.1000004@rowe-clan.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


On 12/31/2006 08:17 PM, William A. Rowe, Jr. wrote:
> Justin Erenkrantz wrote:
> 
>>On 12/30/06, Ruediger Pluem <rpluem@apache.org> wrote:
>>
>>>Digging somewhat deeper it turns out that adding APR_FINFO_NAME to the
>>>list of wanted
>>>information causes this apr_stat to return always APR_INCOMPLETE on
>>>Unix platforms in
>>>the case that the call to the native stat / lstat does not fail. This
>>>raises the
>>>following questions:
>>>
>>>1. Do we need to add APR_FINFO_NAME to this apr_stat call? I think we
>>>do not need it
>>>   on Unix platforms but I am not sure if this is true for other
>>>platforms.
>>
>>resolve_symlink seems to preserve it.  *shrug*
>>
>>
>>>2. If we need it can we simply ignore an APR_INCOMPLETE return code on
>>>all platforms
>>>   and only bail out if ((rv != APR_INCOMPLETE) && (rv != APR_SUCCESS))
>>
>>+1.  -- justin
> 
> 
> +/-1 - Accepting APR_INCOMPLETE was an excellent choice.
> 
> But your patch falls one step short.  It's necessary to inspect fstat.valid
> before we use specific fields.

Yes and no. In general you are right: I missed this check. OTH AFAICT provided
that the native OS stat did not fail for some reason (which will cause the OS
error code returned to rv) apr_stat always returns a valid filetype info on all
platforms. But I am unsure if this is guaranteed by the apr API contract, so it might
be safer to check here:


--- server/request.c    (Revision 491297)
+++ server/request.c    (Arbeitskopie)
@@ -563,7 +563,8 @@
                  * is always going to return APR_INCOMPLETE in the case that
                  * the call to the native stat / lstat did not fail.
                  */
-                if ((rv != APR_INCOMPLETE) && (rv != APR_SUCCESS)) {
+                if (((rv != APR_INCOMPLETE) && (rv != APR_SUCCESS))
+                    || (!(thisinfo.valid & APR_FINFO_TYPE))) {
                     /*
                      * This should never happen, because we did a stat on the
                      * same file, resolving a possible symlink several lines


If this patch is not seen as needed it might make sense to extend the comment
above to explain why it is not needed :-).


Regards

Rüdiger