diff --git a/changes-entries/pr66801.txt b/changes-entries/pr66801.txt new file mode 100644 index 0000000000..5fee4bce91 --- /dev/null +++ b/changes-entries/pr66801.txt @@ -0,0 +1,3 @@ + *) mod_http2: Fix reporting of `Total Accesses` in server-status to not count + HTTP/2 requests twice. Fixes PR 66801. + [Stefan Eissing] diff --git a/modules/http2/h2_mplx.c b/modules/http2/h2_mplx.c index b3138dc4c1..2cfb44478c 100644 --- a/modules/http2/h2_mplx.c +++ b/modules/http2/h2_mplx.c @@ -334,7 +334,6 @@ h2_mplx *h2_mplx_c1_create(int child_num, apr_uint32_t id, h2_stream *stream0, apr_pollset_add(m->pollset, &conn_ctx->pfd); } - m->scratch_r = apr_pcalloc(m->pool, sizeof(*m->scratch_r)); m->max_spare_transits = 3; m->c2_transits = apr_array_make(m->pool, (int)m->max_spare_transits, sizeof(h2_c2_transit*)); @@ -547,16 +546,6 @@ const h2_stream *h2_mplx_c2_stream_get(h2_mplx *m, int stream_id) } -static void c1_update_scoreboard(h2_mplx *m, h2_stream *stream) -{ - if (stream->c2) { - m->scratch_r->connection = stream->c2; - m->scratch_r->bytes_sent = stream->out_frame_octets; - ap_increment_counts(m->c1->sbh, m->scratch_r); - m->scratch_r->connection = NULL; - } -} - static void c1_purge_streams(h2_mplx *m) { h2_stream *stream; @@ -566,8 +555,6 @@ static void c1_purge_streams(h2_mplx *m) stream = APR_ARRAY_IDX(m->spurge, i, h2_stream*); ap_assert(stream->state == H2_SS_CLEANUP); - c1_update_scoreboard(m, stream); - if (stream->input) { h2_beam_destroy(stream->input, m->c1); stream->input = NULL; diff --git a/modules/http2/h2_mplx.h b/modules/http2/h2_mplx.h index ecb4de9cc6..781ddf2f4d 100644 --- a/modules/http2/h2_mplx.h +++ b/modules/http2/h2_mplx.h @@ -99,8 +99,6 @@ struct h2_mplx { struct h2_workers *workers; /* h2 workers process wide instance */ - request_rec *scratch_r; /* pseudo request_rec for scoreboard reporting */ - apr_uint32_t max_spare_transits; /* max number of transit pools idling */ apr_array_header_t *c2_transits; /* base pools for running c2 connections */ }; diff --git a/test/modules/http2/test_008_ranges.py b/test/modules/http2/test_008_ranges.py index 339df1a0cc..dd695bb08d 100644 --- a/test/modules/http2/test_008_ranges.py +++ b/test/modules/http2/test_008_ranges.py @@ -2,6 +2,7 @@ import inspect import json import logging import os +import re import pytest from .env import H2Conf, H2TestEnv @@ -23,10 +24,16 @@ class TestRanges: os.remove(TestRanges.LOGFILE) destdir = os.path.join(env.gen_dir, 'apache/htdocs/test1') env.make_data_file(indir=destdir, fname="data-100m", fsize=100*1024*1024) - conf = H2Conf(env=env) - conf.add([ - "CustomLog logs/test_008 combined" - ]) + conf = H2Conf(env=env, extras={ + 'base': [ + 'CustomLog logs/test_008 combined' + ], + f'test1.{env.http_tld}': [ + '', + ' SetHandler server-status', + '', + ] + }) conf.add_vhost_cgi() conf.add_vhost_test1() conf.install() @@ -134,6 +141,39 @@ class TestRanges: break assert found, f'request not found in {self.LOGFILE}' + # test server-status reporting + # see + def test_h2_008_04(self, env, repeat): + path = '/data-100m' + assert env.apache_restart() == 0 + stats = self.get_server_status(env) + # we see the server uptime check request here + assert 1 == int(stats['Total Accesses']) + assert 1 == int(stats['Total kBytes']) + count = 10 + url = env.mkurl("https", "test1", f'/data-100m?[0-{count-1}]') + r = env.curl_get(url, 5, options=['--http2', '-H', f'Range: bytes=0-{4096}']) + assert r.exit_code == 0, f'{r}' + stats = self.get_server_status(env) + # amount reported is larger than (count *4k), the net payload + # but does not exceed an additional 4k + assert (4*count)+1 <= int(stats['Total kBytes']) + assert (4*(count+1))+1 > int(stats['Total kBytes']) + # total requests is now at 1 from the start, plus the stat check, + # plus the count transfers we did. + assert (2+count) == int(stats['Total Accesses']) + + def get_server_status(self, env): + status_url = env.mkurl("https", "test1", '/status?auto') + r = env.curl_get(status_url, 5) + assert r.exit_code == 0, f'{r}' + stats = {} + for line in r.stdout.splitlines(): + m = re.match(r'([^:]+): (.*)', line) + if m: + stats[m.group(1)] = m.group(2) + return stats + # upload and GET again using curl, compare to original content def curl_upload_and_verify(self, env, fname, options=None): url = env.mkurl("https", "cgi", "/upload.py")