[Bug 61240] apr_file_transfer_contents change breaks htpasswd(1)

[bu...@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=61240

Ruediger Pluem <rp...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|APR                         |support
            Product|APR                         |Apache httpd-2
             Status|NEW                         |RESOLVED
           Keywords|                            |FixedInTrunk,
                   |                            |PatchAvailable
            Version|1.6.2                       |2.5-HEAD
         Resolution|---                         |FIXED
           Assignee|bugs@apr.apache.org         |bugs@httpd.apache.org

--- Comment #6 from Ruediger Pluem <rp...@apache.org> ---
(In reply to Hank Leininger from comment #5)
> (In reply to Ruediger Pluem from comment #4)
> > IMHO APR code is now correct and this is a bug in  htpasswd.
> 
> Yes, I agree that htpasswd.c is using apr_file_copy incorrectly.  And has
> been doing so forever.
> 
> > Does the
> > following patch to htpasswd fix the issue for you?
> > 
> > Index: htpasswd.c
> > ===================================================================
> > --- htpasswd.c  (revision 1800082)
> > +++ htpasswd.c  (working copy)
> > @@ -498,7 +498,7 @@
> > 
> >      /* The temporary file has all the data, just copy it to the new
> > location.
> >       */
> > -    if (apr_file_copy(dirname, pwfilename, APR_FILE_SOURCE_PERMS, pool) !=
> > +    if (apr_file_copy(dirname, pwfilename, APR_OS_DEFAULT, pool) !=
> >          APR_SUCCESS) {
> >          apr_file_printf(errfile, "%s: unable to update file %s" NL,
> >                          argv[0], pwfilename);
> 
> Indeed, that does seem to do the right thing.

Comitted to httpd-trunk as r1800594.

> 
> So every caller of apr_file_copy or apr_file_transfer_contents ought to be
> reviewed to make sure they are not using APR_FILE_SOURCE_PERMS flag (or
> equivalent APR_FPROT_FILE_SOURCE_PERMS) when they do not mean it.
> 
> But in the meantime shouldn't the fix to apr_file_transfer_contents be
> reverted, and instead announced as upcoming so projects that use libapr can
> check?  Unless you can easily tell who all users of apr_file_copy /
> apr_file_transfer_contents are, and can confirm that none of them will have
> this problem leading to surprise failures.  That sounds a) not easy and b)
> not the job of apr-developers?

No, we do not remove a fix for a bug in APR. The documentation here was clear
and it is a clear bug of the caller if it uses APR_FILE_SOURCE_PERMS /
APR_FPROT_FILE_SOURCE_PERMS and if does not want that to happen.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org