| Summary: | Improve error handling during "UNLOCK" | ||
|---|---|---|---|
| Product: | Apache httpd-2 | Reporter: | vijayaguru <vijay> |
| Component: | mod_dav | Assignee: | Apache HTTPD Bugs Mailing List <bugs> |
| Status: | NEW --- | ||
| Severity: | normal | CC: | vijay |
| Priority: | P2 | Keywords: | PatchAvailable |
| Version: | 2.5-HEAD | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Attachments: | patch that improves error handling during "dav unlock" | ||
Created attachment 27095 [details]
patch that improves error handling during "dav unlock"
Attaching the patch that fixes this issue.
This patch does the following things to maintain backward compatibility.
1. Copied dav/main/util_lock.c: dav_unlock() to a static function do_dav_unlock() which accepts "dav_error **err" as an outparam and preserves any error in case of failures.
2. dav_unlock() will call do_dav_unlock() and return HTTP_STATUS code in case of any failures; Otherwise return OK.(Old callers can still use this method)
3.dav_unlock2() will also call do_dav_unlock() and return dav_error type in case of any failures; Otherwise return NULL.
4.dav/main/mod_dav.c: dav_method_unlock() will call dav_unlock2() and *handles* the error in case of any failures.
Updating Keyword as "PatchAvailable". |
While unlocking a file forcibly using subversion client, and if the unlock is blocked by pre-unlock hook script, the client is getting "500 Internal Server Error" instead of the error message sent by the subversion server. Simple reproduction recipe: 1. Lock a file. "svn lock testfile.txt --username user1" 2. Make pre-unlock hook script to fail. 3. Try to unlock the file forcibly as a different user. "svn unlock testfile.txt --force --username user2" step 3 fails with "500 Internal Server Error" message. Whenever the pre-unlock hook fails, Subversion server sends the error message "Unlock blocked by pre-unlock hook (exit code 1)...." to mod_dav. But mod_dav is not handling the error and it is not sending any error messages to client side, instead it sends only HTTP_STATUS code alone(confirmed it with the following codepath). From dav/main/mod_dav.c: dav_method_unlock(), <snip> if ((result = dav_unlock(r, resource, locktoken)) != OK) { return result; } </snip> From dav/main/util_lock.c: dav_unlock(), <snip> err = (*repos_hooks->walk)(&ctx.w, DAV_INFINITY, &multi_status); /* ### fix this! */ /* ### do something with multi_status */ result = err == NULL ? OK : err->status; (*hooks->close_lockdb)(lockdb); return result; </snip> This walk() method finds out the resource to unlock and walker callback functin dav_unlock_walker() removes lock from that resource. From dav/main/util_lock.c: dav_unlock_walker(), <snip> if ((err = (*ctx->w.lockdb->hooks->remove_lock)(ctx->w.lockdb, wres->resource, ctx->locktoken)) != NULL) { /* ### should we stop or return a multistatus? looks like STOP */ /* ### add a higher-level description? */ return err; </snip> In this case, unlock is blocked by pre-unlock hook, subvesion server marshalls the error back to mod_dav, mod_dav's dav_unlock_walker() returns the error sent by svn server, but dav_unlock() returns only the HTTP_STATUS code(err->status) and the error message is *lost* there.