req.sendfile(filename) sends an incorrect number of bytes when filename is a symlink

• Key: MODPYTHON-84
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Graham Dumpleton
• Reporter: Jim Gallacher
• Component/s: core
• Affects Version/s: 3.1.3, 3.1.4, 3.2.7
• Fix Version/s: 3.2.10, 3.3.1

Description

This issue was reported by Wim Heirman on the mod_python list:

When req.sendfile(filename) is called where filename is a symbolic link, only part of the file is sent to the client (as many bytes as there are characters in the symlink's file reference, so for a symlink pointing to '../index.html' returns the first 13 bytes of the correct file).

Wim suggested changing APR_READ to APR_FINFO_NORM in the apr_stat call in req_sendfile().

Comments

[Graham Dumpleton]

As per report by Sergey:

  http://www.modpython.org/pipermail/mod_python/2006-May/021276.html

  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.

with patch:

--- requestobject.c.orig 2006-01-10 19:03:06.000000000 -0500
+++ requestobject.c 2006-05-30 17:11:22.124624100 -0400
@@ -1024,7 +1024,7 @@
 
     Py_BEGIN_ALLOW_THREADS
     status=apr_stat(&finfo, fname,
- APR_FINFO_NORM, self->request_rec->pool);
+ APR_FINFO_SIZE, self->request_rec->pool);
     Py_END_ALLOW_THREADS
     if (status != APR_SUCCESS) {
         PyErr_SetString(PyExc_IOError, "Could not stat file for reading");
@@ -1033,7 +1033,7 @@
     
     Py_BEGIN_ALLOW_THREADS
     status=apr_file_open(&fd, fname,
- APR_READ, finfo.protection,
+ APR_READ, APR_OS_DEFAULT,
                          self->request_rec->pool);
     Py_END_ALLOW_THREADS
     if (status != APR_SUCCESS) {

the original change to fix this symlink issue was causing issues on Win32, plus needs to be changed to work with a APR 1.2.7 change required for subversion to work.

Thus the patch above, seems to be necessary.

At this stage can't see anything wrong with the change.