[mod_python] Tiny patch for better apr_stat usage in sendfile

Jim Gallacher jpg at jgassociates.ca
Tue Jun 6 13:20:38 EDT 2006


Graham Dumpleton wrote:
> Sergey A. Lipnevich wrote ..
>> Hi All,
>>
>> I'd like to offer the following patch for 3.2.8's sendfile function that
>> asks APR to provide less information about a file (which is beneficial
>> on Windows as APR doesn't have to construct POSIX protection flags),
>> because the only field in use becomes file size. Incidentally, it also
>> allows mod_python (plus Apache 2.2 patch) to work with the following
>> tweak for APR 1.2.7 (required for Subversion), under Windows and Apache
>> 2.2.2: http://article.gmane.org/gmane.comp.apache.apr.devel/9389/.
>>
>> I didn't test the patch on anything but Windows XP. The error I was
>> getting with the setup above was "Could not stat file for reading."
>> I also have to mention that the whole thing was compiled with VS 2005
>> compiler to support Python 2.4, although I think it's not relevant for
>> the patch attached.
>>
>> I hope all this patch citing is not too confusing :-).
> 
> Part of this patch may break UNIX systems. Specifically, when target is a
> symbolic link, the size returned will be the length of the filename and
> not the size of the target file when SIZE and not NORM is used. Thus
> an incorrect amount of data is sent back.

I applied this patch on my Linux development machine (apache 2.0.55) 
last week, and have not seen any problems - but then I've not really 
used sendfile in the interim, so why would I? ;)

Mention of the symlink issue twigged a memory of a prior problem, and 
sure enough APR_FINFO_NORM was introduced to fix this very issue.
See http://issues.apache.org/jira/browse/MODPYTHON-84. The patch for 
that issue looks like this:

--- httpd/mod_python/trunk/src/requestobject.c	2005/10/15 15:27:11	321348
+++ httpd/mod_python/trunk/src/requestobject.c	2005/10/15 15:31:46	321349
@@ -1024,7 +1024,7 @@

      Py_BEGIN_ALLOW_THREADS
      status=apr_stat(&finfo, fname,
-                    APR_READ, self->request_rec->pool);
+                    APR_FINFO_NORM, self->request_rec->pool);
      Py_END_ALLOW_THREADS
      if (status != APR_SUCCESS) {
          PyErr_SetString(PyExc_IOError, "Could not stat file for reading");

I don't recall dissecting that bug as it was a one line fix. Looking 
back at that issue, the previous code was incorrectly using APR_READ 
instead of one of the APR_FINFO_* macros. As it happens APR_READ has the 
same value as APR_FINFO_LINK, (which stats the link not the file itself, 
if it is link), thus resulting the reported bug.

We only need to stat the file to get the file size. APR_FINFO_NORM does 
this, but as Sergey points out it drags in alot of unwanted stuff as 
well. As I read it APR_FINFO_SIZE will do what we want, and his patch 
does not introduce a regression. The unit test seems to confirm this.

If this analysis seems correct I'll reopen MODPYTHON-84 and commit 
Sergey's patch against that issue. (I don't see the point in opening a 
new issue). I think this fix would be a good candidate for a backport to 
the 3.2.x branch for inclusion in 3.2.9.

Jim






More information about the Mod_python mailing list