2.4.x mod_cgid fdpassing & mod_cgi/mod_cgid refactoring #209
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
notroj
force-pushed
the
cgid-fdpassing-24x
branch
from
b5def9f
to
798cef0
Compare
notroj
force-pushed
the
cgid-fdpassing-24x
branch
from
2c61338
to
3a0f577
Compare
notroj
force-pushed
the
cgid-fdpassing-24x
branch
from
3a0f577
to
1be960f
Compare
Add experimental support for fd passing in mod_cgid. Attaches CGI script stderr to the error log specific to the vhost, by passing the appropriate fd over the AF_UNIX socket from the request handling thread to the cgid server process. * modules/generators/config5.m4: Add --enable-cgid-fdpassing. * modules/generators/mod_cgid.c (sock_readhdr): New function, also returns auxiliary control data (the stderr fd) if available. (sock_write): Take optional aux fd argument, send it as control data. (send_req, get_req): Adjust accordingly to pass/receive the stderr fd. (cgid_server): Use passed fd if available, limit the lifetime. PR: 60692 Submitted by: jorton
mod_cgid: Continuation of r1862968, experimental fd passing support.
Split out CGI bucket implementation from mod_cgi and use in both
mod_cgi and mod_cgid, bringing stderr handling in mod_cgid up to par
with mod_cgi. (There is a lot of code which has been copied between
mod_cgi{,d} so there's scope for further reduction of source
duplication between the modules using this header)
* modules/generators/cgi_common.h: Copied from mod_cgi.c, removed
everything but the CGI bucket implementation with only one change:
(struct cgi_bucket_data, cgi_bucket_create, cgi_bucket_read): Take a
timeout on bucket creation, store and use on reads.
* modules/generators/mod_cgi.c [APR_FILES_AS_SOCKETS]: Include
cgi_common.h.
(cgi_handler): Pass configured timeout to CGI bucket.
* modules/generators/mod_cgid.c: Include cgi_common.h.
(log_script_err): Copy from mod_cgi.c.
(log_script): Use log_script_err.
(send_req): Take fd for stderr.
(cgid_child_errfn): Handle fd-passing case by writing error
to stderr for client to pass through ap_log_rerror.
(cgid_handler): Create pipe for stderr, pass write-end to
server via send_req, use read-end to create CGI bucket. Handle
stderr output in failure paths.
PR: 54221
Submitted by: jorton
* modules/generators/mod_cgid.c (sock_readhdr): Only set up control message block when required; add some additional error handling. Submitted by: jorton
* modules/generators/cgi_common.h (cgi_bucket_create): Disable APR timeout handling here for all callers. * modules/generators/mod_cgi.c (cgi_handler): ... drop it here. PR: 63797 Submitted by: jorton
Move common (and near-identical) code for CGI response output handling to cgi_common.h; the diff between the modules for this code was as follows: https://people.apache.org/~jorton/mod_cgi-to-cgid-handler.diff Change from previous: mod_cgi will now explicitly discard output when returning HTTP_MOVED_TEMPORARILY for relative redirects (should not be functionally different), TRACE1 logging of ap_pass_brigade failures for mod_cgid is dropped. * modules/generators/cgi_common.h (cgi_handle_response): New function, factored out from mod_cgid. (discard_script_output): Copied function from mod_cgi/d unchanged. * modules/generator/mod_cgid.c (cgid_handler), modules/generator/mod_cgi.c (cgi_handler): Use cgi_handle_response. Submitted by: jorton
Fix build broken w/o --enable-cgid-fdpassing by r1867968: * modules/generators/cgi_common.h: Only define CGI bucket type if WANT_CGI_BUCKET is defined. * modules/generators/mod_cgi.c: Always include cgi_common.h, defining WANT_CGI_BUCKET iff APR_FILES_AS_SOCKETS is defined * modules/generators/mod_cgid.c: Always include cgi_common.h, defining WANT_CGI_BUCKET iff HAVE_CGID_FDPASSING (--enable-cgid-fdpassing). Submitted by: jorton
Add comment, no functional change. Submitted by: jorton
* modules/generators/cgi_common.h (cgi_handle_request): Factor out near-identical common code from mod_cgid, mod_cgi. * modules/generators/mod_cgid.c (cgid_handler), modules/generators/mod_cgi.c (cgi_handler): Adjust to use cgi_handle_request. Github: closes apache#97 Submitted by: jorton
* modules/generators/cgi_common.h (cgi_handle_request): Catch (unlikely) apr_bucket_read() failure when reading request. Submitted by: jorton
* modules/generators/mod_cgid.c (cgid_handler): Bail immediately with a 503 response on errors when talking to the daemon. Check the pid returned is not zero. Submitted by: jorton
* modules/generators/mod_cgid.c (get_req): Add basic sanity checking for the structure received in the CGI daemon. Submitted by: jorton
* modules/generators/mod_cgid.c (get_cgi_pid): Fix test for pid=0. (cgid_handler): Remove duplicated test for pid=0 here added in r1879119. Submitted by: jorton
* modules/generators/cgi_common.h (cgi_handle_response): Avoid trying to read the output brigade twice in the case of a timeout. PR: 64709 Submitted by: jorton
Further re-unification of code duplicated across mod_cgi/mod_cgid into cgi_common.h. Functional changes: - brings the PR 61980 fix to mod_cgid as well, and - some mod_cgid-specific APLOGNOs are dropped in favour of the code used in the equivalent error path in mod_cgi ... otherwise no user-visible changes (intended). * modules/generators/cgi_common.h (log_scripterror, log_script_err): Move here from mod_cgi. (cgi_handle_exec): Move here, renamed from mod_cgi's handle_exec. (cgi_optfns_retrieve): New function, split out from mod_cgi's cgi_post_config. * modules/generators/mod_cgid.c: Adjust accordingly, update to pass logno separately. (register_hooks): Register cgi_optfns_retrieve. * modules/generators/mod_cgi.c: Adjust accordingly. (register_hooks): Register cgi_optfns_retrieve. Github: closes apache#141 Submitted by: jorton
* modules/generators/cgi_common.h (discard_script_output): Simplify slightly and ensure constant rather than unlimited memory consumption when discarding CGI script output (for e.g. a redirect response). Submitted by: jorton
notroj
force-pushed
the
cgid-fdpassing-24x
branch
from
6cb96de
to
c22d82c
Compare
asfgit
pushed a commit
that referenced
this pull request
Jun 17, 2024
…69421, r1874491, r1874511, r1879119, r1879136, r1879860, r1881459, r1881559, r1867970 from trunk:
Add experimental support for fd passing in mod_cgid. Attaches CGI
script stderr to the error log specific to the vhost, by passing the
appropriate fd over the AF_UNIX socket from the request handling
thread to the cgid server process.
* modules/generators/config5.m4: Add --enable-cgid-fdpassing.
* modules/generators/mod_cgid.c (sock_readhdr): New function, also
returns auxiliary control data (the stderr fd) if available.
(sock_write): Take optional aux fd argument, send it as control
data. (send_req, get_req): Adjust accordingly to pass/receive the
stderr fd.
(cgid_server): Use passed fd if available, limit the lifetime.
PR: 60692
mod_cgid: Continuation of r1862968, experimental fd passing support.
Split out CGI bucket implementation from mod_cgi and use in both
mod_cgi and mod_cgid, bringing stderr handling in mod_cgid up to par
with mod_cgi. (There is a lot of code which has been copied between
mod_cgi{,d} so there's scope for further reduction of source
duplication between the modules using this header)
* modules/generators/cgi_common.h: Copied from mod_cgi.c, removed
everything but the CGI bucket implementation with only one change:
(struct cgi_bucket_data, cgi_bucket_create, cgi_bucket_read): Take a
timeout on bucket creation, store and use on reads.
* modules/generators/mod_cgi.c [APR_FILES_AS_SOCKETS]: Include
cgi_common.h.
(cgi_handler): Pass configured timeout to CGI bucket.
* modules/generators/mod_cgid.c: Include cgi_common.h.
(log_script_err): Copy from mod_cgi.c.
(log_script): Use log_script_err.
(send_req): Take fd for stderr.
(cgid_child_errfn): Handle fd-passing case by writing error
to stderr for client to pass through ap_log_rerror.
(cgid_handler): Create pipe for stderr, pass write-end to
server via send_req, use read-end to create CGI bucket. Handle
stderr output in failure paths.
PR: 54221
* modules/generators/mod_cgid.c (sock_readhdr): Only set up control
message block when required; add some additional error handling.
* modules/generators/cgi_common.h (cgi_bucket_create):
Disable APR timeout handling here for all callers.
* modules/generators/mod_cgi.c (cgi_handler): ... drop it here.
PR: 63797
Move common (and near-identical) code for CGI response output handling
to cgi_common.h; the diff between the modules for this code was as
follows:
https://people.apache.org/~jorton/mod_cgi-to-cgid-handler.diff
Change from previous: mod_cgi will now explicitly discard output when
returning HTTP_MOVED_TEMPORARILY for relative redirects (should not be
functionally different), TRACE1 logging of ap_pass_brigade failures
for mod_cgid is dropped.
* modules/generators/cgi_common.h (cgi_handle_response): New function,
factored out from mod_cgid.
(discard_script_output): Copied function from mod_cgi/d unchanged.
* modules/generator/mod_cgid.c (cgid_handler),
modules/generator/mod_cgi.c (cgi_handler): Use cgi_handle_response.
Fix build broken w/o --enable-cgid-fdpassing by r1867968:
* modules/generators/cgi_common.h: Only define CGI bucket type
if WANT_CGI_BUCKET is defined.
* modules/generators/mod_cgi.c: Always include cgi_common.h, defining
WANT_CGI_BUCKET iff APR_FILES_AS_SOCKETS is defined
* modules/generators/mod_cgid.c: Always include cgi_common.h, defining
WANT_CGI_BUCKET iff HAVE_CGID_FDPASSING (--enable-cgid-fdpassing).
Add comment, no functional change.
* modules/generators/cgi_common.h (cgi_handle_request): Factor out
near-identical common code from mod_cgid, mod_cgi.
* modules/generators/mod_cgid.c (cgid_handler),
modules/generators/mod_cgi.c (cgi_handler):
Adjust to use cgi_handle_request.
* modules/generators/cgi_common.h (cgi_handle_request): Catch
(unlikely) apr_bucket_read() failure when reading request.
* modules/generators/mod_cgid.c (cgid_handler): Bail immediately with
a 503 response on errors when talking to the daemon. Check the pid
returned is not zero.
* modules/generators/mod_cgid.c (get_req): Add basic sanity
checking for the structure received in the CGI daemon.
* modules/generators/mod_cgid.c (get_cgi_pid): Fix test for pid=0.
(cgid_handler): Remove duplicated test for pid=0 here added in
r1879119.
* modules/generators/cgi_common.h (cgi_handle_response): Avoid trying
to read the output brigade twice in the case of a timeout.
PR: 64709
Further re-unification of code duplicated across mod_cgi/mod_cgid into
cgi_common.h. Functional changes:
- brings the PR 61980 fix to mod_cgid as well, and
- some mod_cgid-specific APLOGNOs are dropped in favour of the
code used in the equivalent error path in mod_cgi
... otherwise no user-visible changes (intended).
* modules/generators/cgi_common.h (log_scripterror, log_script_err): Move
here from mod_cgi.
(cgi_handle_exec): Move here, renamed from mod_cgi's handle_exec.
(cgi_optfns_retrieve): New function, split out from mod_cgi's cgi_post_config.
* modules/generators/mod_cgid.c: Adjust accordingly, update to pass
logno separately.
(register_hooks): Register cgi_optfns_retrieve.
* modules/generators/mod_cgi.c: Adjust accordingly.
(register_hooks): Register cgi_optfns_retrieve.
* modules/generators/cgi_common.h (discard_script_output): Simplify
slightly and ensure constant rather than unlimited memory
consumption when discarding CGI script output (for e.g. a redirect
response).
Github: closes #209
Reviewed by: jorton, ylavic, covener
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1918379 13f79535-47bb-0310-9956-ffa450edef68
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Merges: 1828172 1862968 1863191 1867878 1867882 1867968 1867971 1869421 1874491 1874511 1879119 1879136 1879860 1881459 1881559 1867970