Bug 30730 - [PatchAvailable] mod_actions and Server-Status
Summary: [PatchAvailable] mod_actions and Server-Status
Status: RESOLVED LATER
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_status (show other bugs)
Version: 2.0.50
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords: MassUpdate, PatchAvailable
Depends on:
Blocks:
 
Reported: 2004-08-18 15:33 UTC by Scott Clark
Modified: 2018-11-07 21:09 UTC (History)
1 user (show)



Attachments
Patch (2.03 KB, patch)
2007-03-13 18:02 UTC, Basant Kumar Kukreja
Details | Diff
patch against trunk (1.49 KB, patch)
2007-03-29 17:35 UTC, Basant Kumar Kukreja
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Scott Clark 2004-08-18 15:33:59 UTC
Hello,

I'me using mod_actions to pass images (and add a watermark) through a php script
using the following in my httpd.conf:
Action image/jpeg /watermark.php

I also use server-status to see how much traffic my box has transfered, when
using this Action field, the Total Traffic stat on /server-status doesn't
represent the full amount of data transfered, just adding a few bytes when an
image is accessed. The data is correct in my log files.

regards,

S :~>
Comment 1 Basant Kumar Kukreja 2007-03-13 14:21:28 UTC
With the following configuration, I am able to reproduce the issue :

ExtendedStatus on
 <Location /server-status>
SetHandler server-status

Order Deny,Allow
Deny from all
Allow from all
</Location>


DocumentRoot "/disk/apache/apache2/imgs"

<Location "/">
    AddHandler watermark .jpeg
    Action watermark "/cgi-bin/watermark1.php"
</Location>

For a request e.g /test.jpeg, request is internally redirected.
ap_internal_redirect_handler handles the redirection.
ap_internal_redirect_handler creates a new request_rec. ap_invoke_handler
process the request. In this case jpeg file is sent. The new request_rec
structure contains the bytes sent but the original request's bytes are never
incremented so mod_status doesn't count the bytes sent.



Here is the watermark1.php
-------------------------------------------

#!/opt/php5.2.1/bin/php
<?php

/*
   program:      makeimg.php
   description:  creates new image by merging
                 source image with watermark
                 and outputs it to browser
*/

header("Content-Type: image/jpeg");
//(because the script outputs picture)


print "\r\n";
$transparency = 40; //watermark's transparency (0-100)

//source photo
$source_photo = stripslashes($_ENV{'PATH_TRANSLATED'});
$photo = imagecreatefromjpeg($source_photo);

//watermark
$watermark = imagecreatefrompng('watermark.png');
$watermark_width = imagesx($watermark);
$watermark_height = imagesy($watermark);

//location of the watermark on the source image
$size = getimagesize($source_photo);
$dest_x = ($size[0] - $watermark_width) / 2;
$dest_y = ($size[1] - $watermark_height) / 2;

//make the image (merge source image with watermark)
imagecopymerge($photo, $watermark, $dest_x, $dest_y, 0, 0, $watermark_width,
$watermark_height, $transparency);

//output the image
imagejpeg($photo);

//free memory
imagedestroy($photo);
imagedestroy($watermark);

?>
-------------------------------------------
php shipped with Fedora Core 5 doesn't work correctly.
Also for this script, cgi/php will not work. cli/php will work correctly
because cgi/php will expect script name to be in PATH_TRANSLATED variable.
-------------------------------------------
Comment 2 Basant Kumar Kukreja 2007-03-13 15:09:23 UTC
If we write a simple php script :

#!/opt/php5.2.1/bin/php
<?php
header('Location: /one.html') ;
header('Status: 200') ;
?>

Then cgi handler cgid_handler invokes ap_internal_redirect_handler which
processes the relocated URI.

In this case too, mod_status doesn't get updated properly because
ap_internal_redirect_handler ignores the new request_rec's monitoring data.

Comment 3 Basant Kumar Kukreja 2007-03-13 18:02:01 UTC
Created attachment 19706 [details]
Patch

ap_increment_counts updates the scoreboard structure (worker_score) and is
called by http processing functions e.g ap_process_http_connection. 

If a request is internally redirected then apache creates a new request_rec
and process the request by calling ap_process_request_internal. Before this
fix, apache miss the bytes sent by a internal redirect. In this fix, we
invoke a new function ap_increment_bytes which only increment the sent bytes. 
It would be incorrect to call ap_increment_counts from internal redirect
function because it increments the access counts too which will result in
more than 1 access count for a single request. A small refactoring was needed
in scoreboard.c to make a single point of incrementing bytes.

List of affected files :
server/scoreboard.c
include/scoreboard.h
modules/http/http_request.c


How did I test the patch :
1. Internal redirect by 
Action image/jpeg /watermark.php
as reported by bug.
2. Internal redirect by php script as mentioned before.

server-status is monitored to make sure that for each access, Total Traffic is
reported correctly. Also "Conn and Child" was also observed for these requests.
Comment 4 Jim Jagielski 2007-03-29 11:12:56 UTC
Why the increment_bytes() function? Why not delete that, and have ap_increment_counts() call 
ap_increment_bytes() directly (passing sb)?
Comment 5 Ruediger Pluem 2007-03-29 12:57:13 UTC
Against which codebase has this patch been created (2.0.x / 2.2.x / trunk)?
Does this problem also occur on trunk? On trunk the cleanup function of the eor
bucket is responsible for incrementing the respective counters and this might
also work in the case of an internal redirect.
Comment 6 Basant Kumar Kukreja 2007-03-29 14:41:51 UTC
The patch is generated against 2.2.x branch.
I will check to see if the issue reproduces on trunk.

Regarding Jim's question "
> Why the increment_bytes() function? Why not delete that, and have
ap_increment_counts() call 
>ap_increment_bytes() directly (passing sb)?
If we call ap_increment_bytes directly then ap_increment_bytes will obtain the
worker_score pointer again by calling :
    ws = &ap_scoreboard_image->servers[sb->child_num][sb->thread_num];

Since ap_increment_count already have the ws pointer so I felt it is bit more
cleaner to have a separate static function which takes worker_score as a
pointer.

Comment 7 Basant Kumar Kukreja 2007-03-29 15:32:42 UTC
I am able to reproduce the problem in trunk too.
Comment 8 Basant Kumar Kukreja 2007-03-29 17:16:42 UTC
I debugged the issue in trunk. eor_bucket_cleanup is getting called. It also
invokes ap_increment_count. But there are two request objects. One is main
request object and another one is the redirected request object which is created
by ap_internal_redirect_handler
    request_rec *new = internal_internal_redirect(new_uri, r);

eor_bucket_cleanup is called with main request object. In the case of
redirect, bytes_sent are stored in second (redirected) request object (named
new).  The result is that access counters are incremented correctly but
bytes_sent never incremented correctly. We can't call ap_increment_counts for
redirected request because in that case access count will be incremented
twice.

$ curl --dump-header - -o /tmp/one.jpeg http://localhost:4014/conv_test.jpeg

Here is the debugging session :

Breakpoint 1, ap_process_request (r=0x96ba458) at http_request.c:278
278         conn_rec *c = r->connection;
(gdb) c
Continuing.

Breakpoint 3, ap_internal_redirect_handler (new_uri=0x96bbf30
"/cgi-bin/watermark1.php/conv_test.jpeg",
    r=0x96ba458) at http_request.c:513
513         request_rec *new = internal_internal_redirect(new_uri, r);
(gdb) n
516         if (!new) {
(gdb) n
520         if (r->handler)
(gdb) n
521             ap_set_content_type(new, r->content_type);
(gdb) n
522         access_status = ap_process_request_internal(new);
(gdb) n
523         if (access_status == OK) {
(gdb) n
524             if ((access_status = ap_invoke_handler(new)) != 0) {
(gdb) n
528             ap_finalize_request_protocol(new);
(gdb) n
533     }
(gdb) p new
$10 = (request_rec *) 0x96bbf58
(gdb) p r
$11 = (request_rec *) 0x96ba458
(gdb) p new->bytes_sent
$12 = 5046
(gdb) p r->bytes_sent
$13 = 0
(gdb) c
Continuing.

Breakpoint 4, eor_bucket_cleanup (data=0x96b47f0) at eor_bucket.c:24
24          apr_bucket *b = (apr_bucket *)data;
(gdb) n
25          request_rec *r = (request_rec *)b->data;
(gdb) n
27          if (r != NULL) {
(gdb) p r
$14 = (request_rec *) 0x96ba458
(gdb) p r->bytes_sent
$15 = 0
(gdb) n
32              b->data = NULL;
(gdb)
34              ap_update_child_status(r->connection->sbh, SERVER_BUSY_LOG, r);
(gdb)
35              ap_run_log_transaction(r);
(gdb)
36              if (ap_extended_status) {
(gdb)
37                  ap_increment_counts(r->connection->sbh, r);
(gdb)
40          return APR_SUCCESS;
(gdb) p r->bytes_sent
$16 = 0
(gdb) c

I considered transferring for bytes from redirected request object to main
request object but I think that might create other problems.
Comment 9 Basant Kumar Kukreja 2007-03-29 17:35:17 UTC
Created attachment 19843 [details]
patch against trunk

Patch generated for httpd trunk.

Before the fix :

Parent Server Generation: 0
Server uptime: 3 seconds
Total accesses: 0 - Total Traffic: 0 kB
CPU Usage: u0 s0 cu0 cs0
0 requests/sec - 0 B/second -
---------------------------
After sending 4 jpeg requests (5 KB each)
---------------------------
Parent Server Generation: 0
Server uptime: 58 seconds
Total accesses: 5 - Total Traffic: 2 kB
CPU Usage: u0 s0 cu0 cs0
.0862 requests/sec - 35 B/second - 409 B/request

Note that access count is incremented but bytes count remained 2 kB while it
should be 
4 * 5kB + 2kB for status = 22 kB

--------------------------------------------------------------------
After the fix :

Parent Server Generation: 2
Server uptime: 1 second
Total accesses: 0 - Total Traffic: 0 kB
CPU Usage: u0 s0 cu0 cs0
0 requests/sec - 0 B/second -

---------------------------
After sending 4 jpeg requests (5 KB each)
---------------------------
Parent Server Generation: 2
Server uptime: 35 seconds
Total accesses: 5 - Total Traffic: 22 kB
CPU Usage: u0 s0 cu0 cs0
.143 requests/sec - 643 B/second - 4505 B/request

Note that traffic is now 22 kB

--------------------------------------------------------------------
Comment 10 William A. Rowe Jr. 2018-11-07 21:09:14 UTC
Please help us to refine our list of open and current defects; this is a mass update of old and inactive Bugzilla reports which reflect user error, already resolved defects, and still-existing defects in httpd.

As repeatedly announced, the Apache HTTP Server Project has discontinued all development and patch review of the 2.2.x series of releases. The final release 2.2.34 was published in July 2017, and no further evaluation of bug reports or security risks will be considered or published for 2.2.x releases. All reports older than 2.4.x have been updated to status RESOLVED/LATER; no further action is expected unless the report still applies to a current version of httpd.

If your report represented a question or confusion about how to use an httpd feature, an unexpected server behavior, problems building or installing httpd, or working with an external component (a third party module, browser etc.) we ask you to start by bringing your question to the User Support and Discussion mailing list, see [https://httpd.apache.org/lists.html#http-users] for details. Include a link to this Bugzilla report for completeness with your question.

If your report was clearly a defect in httpd or a feature request, we ask that you retest using a modern httpd release (2.4.33 or later) released in the past year. If it can be reproduced, please reopen this bug and change the Version field above to the httpd version you have reconfirmed with.

Your help in identifying defects or enhancements still applicable to the current httpd server software release is greatly appreciated.