Merge ~michal-maloszewski99/ubuntu/+source/apache2:apache2-jammy-fix-proxy-hcheck into ubuntu/+source/apache2:ubuntu/jammy-devel

Proposed by Michał Małoszewski
Status: Merged
Approved by: git-ubuntu bot
Approved revision: not available
Merge reported by: Michał Małoszewski
Merged at revision: 5f7bbf7808649d572b9f51b7fed7fe299ae1018f
Proposed branch: ~michal-maloszewski99/ubuntu/+source/apache2:apache2-jammy-fix-proxy-hcheck
Merge into: ubuntu/+source/apache2:ubuntu/jammy-devel
Diff against target: 86 lines (+64/-0)
3 files modified
debian/changelog (+7/-0)
debian/patches/mod_proxy_hcheck_jammy_fix_to_detect_support.patch (+56/-0)
debian/patches/series (+1/-0)
Reviewer Review Type Date Requested Status
git-ubuntu bot Approve
Bryce Harrington (community) Approve
Canonical Server Reporter Pending
Review via email:
To post a comment you must log in.
Revision history for this message
Michał Małoszewski (michal-maloszewski99) wrote :
Revision history for this message
Michał Małoszewski (michal-maloszewski99) wrote :

Working on SRU template right now

Revision history for this message
Michał Małoszewski (michal-maloszewski99) wrote :
Revision history for this message
Bryce Harrington (bryce) wrote :
Download full text (3.1 KiB)

Thanks for including the draft of the SRU text. I've got a few suggestions but they won't affect the review of the packaging so will give them separate.


What the SRU team looks for here is more of an explanation of why the bug deserves to be accepted as SRU. So, explaining how it impacts end users - i.e. how it disrupts their usage of the software - is important. Sometimes that requires explaining acronyms or other jargon that may not be generally known by Ubuntu developers; Wikipedia often comes to the rescue here *grin*.

However, at the same time I'd avoid using judgement words like "impossible", "unusable", "randomly", etc. that bug reporters frequently tend to use. They might mean one thing to one person, and something different to another. Usually these can be restated in a more precise way without too much difficulty.

One non-obvious thing about the impact section, is that the SRU team doesn't really care about how the bug occurs or how it is fixed, that's almost like an implementational detail. They suggest moving that to a [Discussion] section, particularly if the explanation is wordy, but if possible I usually try to include a brief statement about how the issue is fixed.

So, with all that, I'd suggest editing the impact section a bit:

The Apache JServe Protocol (AJP) proxies inbound requests to an application server, such as health checks via CPING/CPONG. mod_proxy_hcheck added support for AJP/CPING in 22.04, however the following error is encountered when enabling it, which prevents the new feature from operating:

   BalancerMember Health check method CPING not (yet) implemented

This is caused by a incorrect check for AJP support in hc_post_config() that occurs too late, after the configuration syntax has already been marked invalid by the time the "hcmethod=CPING" token is found. The fix is to move the check from hc_post_config() to hc_pre_config().

[Where problems could occur]

What you've written is all true, but much of that is generically true of any SRU. So, I don't think it hurts to include it, but I think what the SRU team is looking for is more about what is distinctive of *this* SRU.

Often I like to think of this section as answering the question, if I were an Apache2 user and updated to this version of the package, and something weird started happening, what types of bugs would I reasonably suspect are this update's fault, vs. ones that are likely to be unrelated? Usually to answer that you consider what the patch actually does, and if there was an undiscovered error in the code what kinds of failures would we expect to see?

So in this case, the patch itself modifies the code of mod_proxy_hcheck, so any new bugs involving that module would be suspect. The patch changes configuration code, so issues cropping up that seem related to module configuration could be suspect. Finally, since the patch modifies C code, issues typical of C code (segfaults, memory leaks, etc.) would be plausible, however since this moves a chunk of code unmodified this seems extremely unlikely.

[Test Plan]
The test case looks great. I'll run through it while doing the packaging review, but on read throug...


Revision history for this message
Bryce Harrington (bryce) wrote :

One thing I'd recommend when filing MP's, is to include a link to the PPA in your MP description, just as a convenience.

You only need one PPA, not two. A PPA can hold multiple packages each for a different Ubuntu release. So you can upload both the kinetic and jammy versions of your SRU package to the same PPA. Doing it with two PPAs doesn't cause any problems, so it's ok, but by convention we typically have a single PPA per SRU.

I would also recommend putting the PPA installation directions into the [Test Case]. I.e.:

    $ sudo add-apt-repository -yus ppa:michal-maloszewski99/apache2-ppa-jammy-lp2003189
    $ sudo apt-get -y upgrade

I would put that in the "Example of successful output" section.

Revision history for this message
Bryce Harrington (bryce) wrote (last edit ):

When I ran the [Test Case], on both kinetic and jammy, there seems to be an error in the config snippet, even with your PPA installed I got an error:

root@kinetic-test:~# apachectl -t
(2)No such file or directory: AH02291: Cannot access directory '/etc/apache2/logs/' for error log of vhost defined at /etc/apache2/sites-enabled/httpd-hcheck-ajp.conf:1
AH00014: Configuration check failed
Action '-t' failed.
The Apache error log may have more information.

To fix this, I changed the snippet to prefix APACHE_LOG_DIR, like this:

<VirtualHost *:80>
    ServerAdmin <email address hidden>
    DocumentRoot "/var/www/html"
    ErrorLog "${APACHE_LOG_DIR}/"
    CustomLog "${APACHE_LOG_DIR}/" common

    <Proxy balancer://myapp>
        BalancerMember ajp:// route=app-route timeout=300 ping=3 connectiontimeout=3 hcmethod=CPING

    ProxyPass /myapp balancer://myapp stickysession=JSESSIONID


Did the test case work ok for you with just 'ErrorLog “logs/”'? From what I've googled, it looks like that should work but it didn't for me.

Revision history for this message
Bryce Harrington (bryce) wrote :

Turning to the package, the packaging is solid, good work.

I've sponsored the upload:

stirling: ~/pkg/Apache2/review-lp1998311/apache2-gu$ ls ../*source.changes
../apache2_2.4.52-1ubuntu4.4_source.changes ../apache2_2.4.54-2ubuntu1.2_source.changes
stirling: ~/pkg/Apache2/review-lp1998311/apache2-gu$ grep ^Vcs* ../*source.changes
../apache2_2.4.52-1ubuntu4.4_source.changes:Vcs-Git-Commit: 94e5eb899badd54bb4004c1ab66c54218c5f1c94
../apache2_2.4.52-1ubuntu4.4_source.changes:Vcs-Git-Ref: refs/heads/apache2-jammy-fix-proxy-hcheck
../apache2_2.4.54-2ubuntu1.2_source.changes:Vcs-Git-Commit: 15929b2d6a8c8a9e239f081d5254555c5e4e57f4
../apache2_2.4.54-2ubuntu1.2_source.changes:Vcs-Git-Ref: refs/heads/apache2-kinetic-fix-proxy-hcheck
stirling: ~/pkg/Apache2/review-lp1998311/apache2-gu$ dput ubuntu ../apache2_2.4.52-1ubuntu4.4_source.changes
D: Setting host argument.
Checking signature on .changes
gpg: ../apache2_2.4.52-1ubuntu4.4_source.changes: Valid signature from E603B2578FB8F0FB
Checking signature on .dsc
gpg: ../apache2_2.4.52-1ubuntu4.4.dsc: Valid signature from E603B2578FB8F0FB
Uploading to ubuntu (via ftp to
  Uploading apache2_2.4.52-1ubuntu4.4.dsc: done.
  Uploading apache2_2.4.52-1ubuntu4.4.debian.tar.xz: done.
  Uploading apache2_2.4.52-1ubuntu4.4_source.buildinfo: done.
  Uploading apache2_2.4.52-1ubuntu4.4_source.changes: done.
Successfully uploaded packages.
stirling: ~/pkg/Apache2/review-lp1998311/apache2-gu$ dput ubuntu ../apache2_2.4.54-2ubuntu1.2_source.changes
D: Setting host argument.
Checking signature on .changes
gpg: ../apache2_2.4.54-2ubuntu1.2_source.changes: Valid signature from E603B2578FB8F0FB
Checking signature on .dsc
gpg: ../apache2_2.4.54-2ubuntu1.2.dsc: Valid signature from E603B2578FB8F0FB
Uploading to ubuntu (via ftp to
  Uploading apache2_2.4.54-2ubuntu1.2.dsc: done.
  Uploading apache2_2.4.54-2ubuntu1.2.debian.tar.xz: done.
  Uploading apache2_2.4.54-2ubuntu1.2_source.buildinfo: done.
  Uploading apache2_2.4.54-2ubuntu1.2_source.changes: done.
Successfully uploaded packages.

You can update the SRU text as you wish, and when ready go ahead and subscribe the SRU team to the bug in order to proceed with the next steps.

review: Approve
Revision history for this message
git-ubuntu bot (git-ubuntu-bot) wrote :

Approvers: bryce, michal-maloszewski99
Uploaders: bryce
MP auto-approved

review: Approve
Revision history for this message
Michał Małoszewski (michal-maloszewski99) wrote :

Version changed from 2.4.52-1ubuntu4.4 to 2.4.52-1ubuntu4.5.
Autopkgtests work fine.

Results: (from
  apache2 @ amd64:
    14.03.23 20:22:08 ✅ Triggers: apache2/2.4.52-1ubuntu4.5~ppa3
  apache2 @ arm64:
    14.03.23 20:35:56 ✅ Triggers: apache2/2.4.52-1ubuntu4.5~ppa3
  apache2 @ armhf:
    14.03.23 20:12:41 ✅ Triggers: apache2/2.4.52-1ubuntu4.5~ppa3
  apache2 @ ppc64el:
    14.03.23 19:54:11 ✅ Triggers: apache2/2.4.52-1ubuntu4.5~ppa3
  apache2 @ s390x:
    14.03.23 20:04:59 ✅ Triggers: apache2/2.4.52-1ubuntu4.5~ppa3

Revision history for this message
Michał Małoszewski (michal-maloszewski99) wrote :

Needs to be sponsored again.

Revision history for this message
Bryce Harrington (bryce) wrote :

You need to set the status of this MP to Merged, Michal.

Revision history for this message
Michał Małoszewski (michal-maloszewski99) wrote :

Done, thx Bryce

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 6f0da90..56903d8 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,10 @@
6+apache2 (2.4.52-1ubuntu4.5) jammy; urgency=medium
8+ * d/p/mod_proxy_hcheck_jammy_fix_to_detect_support.patch: Fix issue
9+ where enabling mod_proxy_hcheck results in error (LP: #1998311)
11+ -- Michal Maloszewski <> Wed, 01 Mar 2023 23:43:55 +0100
13 apache2 (2.4.52-1ubuntu4.4) jammy-security; urgency=medium
15 * SECURITY UPDATE: HTTP request splitting with mod_rewrite and mod_proxy
16diff --git a/debian/patches/mod_proxy_hcheck_jammy_fix_to_detect_support.patch b/debian/patches/mod_proxy_hcheck_jammy_fix_to_detect_support.patch
17new file mode 100644
18index 0000000..e3b79f5
19--- /dev/null
20+++ b/debian/patches/mod_proxy_hcheck_jammy_fix_to_detect_support.patch
21@@ -0,0 +1,56 @@
22+From eaafacd0efc6bae4e2a9de616bf487b8a55437c1 Mon Sep 17 00:00:00 2001
23+From: Eric Covener <>
24+Date: Tue, 29 Nov 2022 13:24:16 +0000
25+Subject: [PATCH] Merge r1904516 from trunk:
26+Description: Fix the mod_proxy_hcheck module
27+Origin: backport,
30+Last-Update: 2022-03-01
32+ modules/proxy/mod_proxy_hcheck.c | 23 ++++++++++++-----------
33+ 1 file changed, 12 insertions(+), 11 deletions(-)
35+--- a/modules/proxy/mod_proxy_hcheck.c
36++++ b/modules/proxy/mod_proxy_hcheck.c
37+@@ -1070,6 +1070,18 @@
38+ hctp = NULL;
39+ tpsize = HC_THREADPOOL_SIZE;
40+ #endif
42++ ajp_handle_cping_cpong = APR_RETRIEVE_OPTIONAL_FN(ajp_handle_cping_cpong);
43++ if (ajp_handle_cping_cpong) {
44++ proxy_hcmethods_t *method = proxy_hcmethods;
45++ for (; method->name; method++) {
46++ if (method->method == CPING) {
47++ method->implemented = 1;
48++ break;
49++ }
50++ }
51++ }
53+ return OK;
54+ }
55+ static int hc_post_config(apr_pool_t *p, apr_pool_t *plog,
56+@@ -1126,17 +1138,6 @@
57+ s = s->next;
58+ }
60+- ajp_handle_cping_cpong = APR_RETRIEVE_OPTIONAL_FN(ajp_handle_cping_cpong);
61+- if (ajp_handle_cping_cpong) {
62+- proxy_hcmethods_t *method = proxy_hcmethods;
63+- for (; method->name; method++) {
64+- if (method->method == CPING) {
65+- method->implemented = 1;
66+- break;
67+- }
68+- }
69+- }
71+ return OK;
72+ }
74+--- /dev/null
75++++ b/mod_proxy_hcheck_jammy_fix_to_detect_support.patch
76+@@ -0,0 +1 @@
78diff --git a/debian/patches/series b/debian/patches/series
79index ff7092d..305f31a 100644
80--- a/debian/patches/series
81+++ b/debian/patches/series
82@@ -31,3 +31,4 @@ CVE-2022-37436.patch
83 CVE-2023-25690-1.patch
84 CVE-2023-25690-2.patch
85 CVE-2023-27522.patch


People subscribed via source and target branches