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 2007/09/13 17:09:53 UTC
DO NOT REPLY [Bug 43386] New: - Default handler produces wrong content length when replacing file
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=43386>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=43386
Summary: Default handler produces wrong content length when
replacing file
Product: Apache httpd-2
Version: 2.2.4
Platform: Sun
OS/Version: Solaris
Status: NEW
Severity: normal
Priority: P2
Component: Core
AssignedTo: bugs@httpd.apache.org
ReportedBy: anathaniel@apache.org
There is a race condition in the default handler which results in the content
being truncated, if the file is replaced during the request.
AFAIU in default_handler the following code is executed (using simple IO for
demonstration):
stat(path, &finfo);
int fd = open(path, ...);
read(fd, buf, finfo.size);
If the file is replaced between the stat and the open calls, then the default
handler uses the size of the old file as content length header and reads the
corresponding number of bytes from the new file.
So if the new file is larger, then the content is truncated. If it is
smaller, then ???
To avoid this race condition, the file should be opened first and the open
file handle be passed to fstat(). Alternatively, use chunked encoding to
avoid having to calculate the content length in advance.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 43386] - Default handler produces wrong content length when replacing file
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=43386>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=43386
------- Additional Comments From wrowe@apache.org 2007-09-17 04:36 -------
rahul, the httpd project has acknowledged the race for the past seven or more
years, but there is simply nothing to be done about it.
We are likely to move to, as you suggest, stat'ing the fd and not trusting
the resource-by-name in the next iteration of httpd (e.g. 2.4 or 3.0). But
there will *still* be a race if the file is truncated during the send.
As Joe suggests, if you are modifying your content on the fly, you will
encounter side effects (always). The only issue are the specific side effects
based on how you are manipulating that content and how many extra steps httpd
performs to work around you.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 43386] - Default handler produces wrong content length when replacing file
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=43386>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=43386
------- Additional Comments From rahul@sun.com 2007-09-17 02:54 -------
This is easily reproducible, but is hard to fix, The reason below.
The finfo structure is populated in ap_directory_walk
---------------------------
r->filename = entry_dir;
cache = prep_walk_cache(AP_NOTE_DIRECTORY_WALK, r);
/* If this is not a dirent subrequest with a preconstructed
* r->finfo value, then we can simply stat the filename to
* save burning mega-cycles with unneeded stats - if this is
* an exact file match. We don't care about failure... we
* will stat by component failing this meager attempt.
*
* It would be nice to distinguish APR_ENOENT from other
* types of failure, such as APR_ENOTDIR. We can do something
* with APR_ENOENT, knowing that the path is good.
*/
if (!r->finfo.filetype || r->finfo.filetype == APR_LNK) {
=> rv = apr_stat(&r->finfo, r->filename, APR_FINFO_MIN, r->pool);
---------------------------
]where
=>[1] ap_directory_walk(r = 0x2ba850), line 508 in "request.c"
[2] core_map_to_storage(r = 0x2ba850), line 3471 in "core.c"
[3] ap_run_map_to_storage(0x2ba850, 0x14, 0x0, 0xa, 0x0, 0x0), at 0x82cf0
[4] ap_process_request_internal(r = 0x2ba850), line 150 in "request.c"
[5] ap_process_async_request(r = 0x2ba850), line 242 in "http_request.c"
[6] ap_process_request(r = 0x2ba850), line 288 in "http_request.c"
===========================================================
While, we open the FD in core
-------------------------
if (r->method_number != M_GET) {
core_request_config *req_cfg;
req_cfg = ap_get_module_config(r->request_config, &core_module);
if (!req_cfg->deliver_script) {
/* The flag hasn't been set for this request. Punt. */
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"This resource does not accept the %s method.",
r->method);
return HTTP_METHOD_NOT_ALLOWED;
}
}
=> if ((status = apr_file_open(&fd, r->filename, APR_READ | APR_BINARY
#if APR_HAS_SENDFILE
| ((d->enable_sendfile == ENABLE_SENDFILE_OFF)
? 0 : APR_SENDFILE_ENABLED)
#endif
, 0, r->pool)) != APR_SUCCESS) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r,
"file permissions deny server access: %s", r->filename);
return HTTP_FORBIDDEN;
}
ap_update_mtime(r, r->finfo.mtime);
ap_set_last_modified(r);
ap_set_etag(r);
apr_table_setn(r->headers_out, "Accept-Ranges", "bytes");
ap_set_content_length(r, r->finfo.size);
---------------------------
]where
=>[1] default_handler(r = 0x2ba850), line 3600 in "core.c"
[2] ap_run_handler(0x2ba850, 0x3b3b3b3b, 0x6c000000, 0x80808080, 0xff00,
0x80808080), at 0x8ba30
[3] ap_invoke_handler(r = 0x2ba850), line 378 in "config.c"
[4] ap_process_async_request(r = 0x2ba850), line 244 in "http_request.c"
[5] ap_process_request(r = 0x2ba850), line 288 in "http_request.c"
While this is a bug, I think there might be other methods that rely on finfo
that get called in between, and some of them may lead to other request paths
that do not require the FD. With out significant patching, the best option may
be to redo the ap_(f)stat again once the fd is opened.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 43386] - Default handler produces wrong content length when replacing file
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=43386>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=43386
rahul@sun.com changed:
What |Removed |Added
----------------------------------------------------------------------------
OS/Version|Solaris |All
Platform|Sun |All
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 43386] - Default handler produces wrong content length when replacing file
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=43386>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=43386
rahul@sun.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |REOPENED
Resolution|INVALID |
------- Additional Comments From rahul@sun.com 2007-09-17 03:56 -------
(In reply to comment #2)
It is acutally relevant for this bug, since the race condition is reported
'when replacing the file' not because the poster is modifying the file in
between.
The atomic operations that you have described does not help in this case.
ie:
-> apr_stat called on file
.....
<- rename/replace atomic [file replaced by file.new]
-> apr_file_open file (opens fd.new instead of fd)
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 43386] - Default handler produces wrong content length when replacing file
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=43386>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=43386
------- Additional Comments From anathaniel@apache.org 2007-09-19 00:35 -------
AFAICS rahuls patch is all that is needed. It updates r->finfo that Content-
Length, Last-Modified, and ETag headers will be consistent with the content.
Even if the file is replaced concurrently, the request will either return the
old file or the new file.
For me this atomicity is a vital property which for the last ten years I took
for granted to be implemented in httpd. Sure, if the file is truncated, then
all bets are off. But if the content provider goes to the trouble to use an
atomic replace, then httpd should reflect that to the http client.
I am astonished that this seems to be a wellknown long-term bug rather than a
regression.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 43386] - Default handler produces wrong content length when replacing file
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=43386>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=43386
------- Additional Comments From rahul@sun.com 2007-09-17 03:35 -------
Created an attachment (id=20832)
--> (http://issues.apache.org/bugzilla/attachment.cgi?id=20832&action=view)
Patch to double check the content length
This patch checks the content length again after the FD has been opened using
file_info_get(fd). This is probably not the best way forward since it is a
double
check (as described in the comments), but it will solve the race condition.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 43386] - Default handler produces wrong content length when replacing file
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=43386>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=43386
jorton@redhat.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |INVALID
------- Additional Comments From jorton@redhat.com 2007-09-17 02:59 -------
The order of the open/stat calls it not really relevant; there will always be a
race between when the file is stat'ed and when it is served. The way around
this is to avoid modifying files in-place in your document root; ensure they are
updated only using atomic operations ("write content to temporary file; rename
file into place").
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org