You are viewing a plain text version of this content. The canonical link for it is here.
Posted to bugs@httpd.apache.org by bu...@apache.org on 2012/11/14 23:48:09 UTC

[Bug 54145] New: Poor error handling in dav_method_put()

https://issues.apache.org/bugzilla/show_bug.cgi?id=54145

          Priority: P2
            Bug ID: 54145
          Assignee: bugs@httpd.apache.org
           Summary: Poor error handling in dav_method_put()
          Severity: minor
    Classification: Unclassified
                OS: All
          Reporter: ben@reser.org
          Hardware: All
            Status: NEW
           Version: 2.5-HEAD
         Component: mod_dav
           Product: Apache httpd-2

dav_method_put() in the mod_dav module has several issues with error handling.

1) The error reported when receiving an error from ap_get_brigade() is "Could
not get next brigade" which is utterly useless for an end user in understanding
what happened.  Further on down the error message generated for an error from
apr_bucket_read() is "An error occurred while reading the request body".  

The call to ap_get_brigade() is where the actual read of the request body
happens.  So if there is any error actually reading the data from the socket
(e.g. socket closes prematurely) then the "Could not get next brigade" error
will be logged.  The apr_bucket_read() is actually reading from bucket which
was created from the ap_get_brigade() call and is very unlikely to fail since
it's essentially just reading data in memory.

2) It does consumes and does not log the error messages from the DAV provider
module that is handling the actual actions that the PUT represents when there
is also an error handling the protocol.  In many cases these errors may be more
useful than the generic HTTP protocol error.  

Consider an SVN client where the PUT is sending an svndiff.  For some reason
the connection is interrupted.  "Could not get next brigade" is logged since
that's the error that mod_dav had, but when mod_dav calls the close_stream
function on the mod_dav_svn module it gets back "Unexpected end of svndiff
input", which is incredibly useful since it implies to the end user that the
connection was interrupted somehow.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54145] Poor error handling in dav_method_put()

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54145

Jeff Trawick <tr...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |FixedInTrunk

--- Comment #5 from Jeff Trawick <tr...@apache.org> ---
Trunk r1464241

httpd versioning rules allow new APIs to be added to stable branches.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54145] Poor error handling in dav_method_put()

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54145

--- Comment #1 from Ben Reser <be...@reser.org> ---
Created attachment 29598
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=29598&action=edit
Improves error handling in dav_method_put()

The attached patch resolves the problems described on this bug.

1) The error message for the ap_get_brigade() is changed to "An error occurred
while reading the request body" and the later apr_bucket_read() calls error
message is changed to be unique by adding the phrase "from the bucket."  Since
the  apr_bucket_read() error is highly unlikely the more technical and internal
oriented error message is appropriate.

2) A new function dav_join_error() is created that allows one error stack to be
added to the end of another.  This is used in non-looping portions of
dav_method_put to preserve both the mod_dav error and the DAV providers error.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54145] Poor error handling in dav_method_put()

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54145

--- Comment #2 from Ben Reser <be...@reser.org> ---
Created attachment 29599
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=29599&action=edit
gdb-script to simulate error from ap_get_brigade()

I tested this using gdb to trigger an error.

The gdb-script attachment allows you to simulate a failure during a PUT.  It's
setup to fail on the 2nd PUT it receives after 400 calls to ap_get_brigade(). 
I was sending PUTs that were 2MB in size to trigger the failure.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54145] Poor error handling in dav_method_put()

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54145

--- Comment #6 from Graham Leggett <mi...@sharp.fm> ---
Proposed for backport to v2.4.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54145] Poor error handling in dav_method_put()

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54145

Christophe JAILLET <ch...@wanadoo.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54145] Poor error handling in dav_method_put()

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54145

--- Comment #3 from Ben Reser <be...@reser.org> ---
If it's desired to backport this to previous httpd releases the addition of the
dav_join_error() function would violate API versioning guarantees.  So this
function should be adjusted to be a private function.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54145] Poor error handling in dav_method_put()

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54145

--- Comment #7 from Graham Leggett <mi...@sharp.fm> ---
Backported to v2.4.5.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54145] Poor error handling in dav_method_put()

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54145

Ben Reser <be...@reser.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |PatchAvailable

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 54145] Poor error handling in dav_method_put()

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54145

--- Comment #4 from Ben Reser <be...@reser.org> ---
Forgot to mention I moved the apr_bucket_read() in that patch inside the test
to see if there is an error before writing.  There's no point in getting the
data in the bucket if we're not going to write it.  We continue to iterate the
bucket so that we can find the EOS and not break the connection in the case of
keep alives.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org