You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2023/04/14 14:02:12 UTC
svn commit: r1909135 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h server/core.c
Author: minfrin
Date: Fri Apr 14 14:02:11 2023
New Revision: 1909135
URL: http://svn.apache.org/viewvc?rev=1909135&view=rev
Log:
core: Be explicit if an enclosing directive contains a path or a
regex.
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/include/ap_mmn.h
httpd/httpd/trunk/server/core.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1909135&r1=1909134&r2=1909135&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Fri Apr 14 14:02:11 2023
@@ -1,6 +1,9 @@
-*- coding: utf-8 -*-
Changes with Apache 2.5.1
+ *) core: Be explicit if an enclosing directive contains a path or a
+ regex. [Graham Leggett]
+
*) mod_http2: fixed a crash during connection termination. See PR 66539.
[Stefan Eissing]
Modified: httpd/httpd/trunk/include/ap_mmn.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=1909135&r1=1909134&r2=1909135&view=diff
==============================================================================
--- httpd/httpd/trunk/include/ap_mmn.h (original)
+++ httpd/httpd/trunk/include/ap_mmn.h Fri Apr 14 14:02:11 2023
@@ -714,6 +714,7 @@
* 20211221.9 (2.5.1-dev) Add additional hcmethod_t enums and PROXY_WORKER_IS_ERROR
* 20211221.10 (2.5.1-dev) Add ap_proxy_canonenc_ex
* 20211221.11 (2.5.1-dev) Add AP_CTIME_OPTION_GMTOFF to util_time.h
+ * 20211221.12 (2.5.1-dev) Add cmd_parms->regex
*/
#define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
@@ -721,7 +722,7 @@
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 20211221
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 11 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 12 /* 0...n */
/**
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a
Modified: httpd/httpd/trunk/server/core.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1909135&r1=1909134&r2=1909135&view=diff
==============================================================================
--- httpd/httpd/trunk/server/core.c (original)
+++ httpd/httpd/trunk/server/core.c Fri Apr 14 14:02:11 2023
@@ -2515,7 +2515,6 @@ static const char *dirsection(cmd_parms
char *old_path = cmd->path;
core_dir_config *conf;
ap_conf_vector_t *new_dir_conf = ap_create_per_dir_config(cmd->pool);
- ap_regex_t *r = NULL;
const command_rec *thiscmd = cmd->cmd;
const char *err = ap_check_cmd_context(cmd, NOT_IN_DIR_CONTEXT);
@@ -2538,16 +2537,17 @@ static const char *dirsection(cmd_parms
if (!strcmp(cmd->path, "~")) {
cmd->path = ap_getword_conf(cmd->pool, &arg);
- if (!cmd->path)
+ if (!cmd->path) {
return "<Directory ~ > block must specify a path";
- r = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED|USE_ICASE);
- if (!r) {
+ }
+ cmd->regex = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED|USE_ICASE);
+ if (!cmd->regex) {
return "Regex could not be compiled";
}
}
else if (thiscmd->cmd_data) { /* <DirectoryMatch> */
- r = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED|USE_ICASE);
- if (!r) {
+ cmd->regex = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED|USE_ICASE);
+ if (!cmd->regex) {
return "Regex could not be compiled";
}
}
@@ -2587,13 +2587,13 @@ static const char *dirsection(cmd_parms
if (errmsg != NULL)
return errmsg;
- conf->r = r;
+ conf->r = cmd->regex;
conf->d = cmd->path;
conf->d_is_fnmatch = (apr_fnmatch_test(conf->d) != 0);
- if (r) {
+ if (cmd->regex) {
conf->refs = apr_array_make(cmd->pool, 8, sizeof(char *));
- ap_regname(r, conf->refs, AP_REG_MATCH, 1);
+ ap_regname(cmd->regex, conf->refs, AP_REG_MATCH, 1);
}
/* Make this explicit - the "/" root has 0 elements, that is, we
@@ -2625,7 +2625,6 @@ static const char *urlsection(cmd_parms
int old_overrides = cmd->override;
char *old_path = cmd->path;
core_dir_config *conf;
- ap_regex_t *r = NULL;
const command_rec *thiscmd = cmd->cmd;
ap_conf_vector_t *new_url_conf = ap_create_per_dir_config(cmd->pool);
const char *err = ap_check_cmd_context(cmd, NOT_IN_DIR_CONTEXT);
@@ -2647,15 +2646,15 @@ static const char *urlsection(cmd_parms
cmd->override = OR_ALL|ACCESS_CONF;
if (thiscmd->cmd_data) { /* <LocationMatch> */
- r = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED);
- if (!r) {
+ cmd->regex = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED);
+ if (!cmd->regex) {
return "Regex could not be compiled";
}
}
else if (!strcmp(cmd->path, "~")) {
cmd->path = ap_getword_conf(cmd->pool, &arg);
- r = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED);
- if (!r) {
+ cmd->regex = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED);
+ if (!cmd->regex) {
return "Regex could not be compiled";
}
}
@@ -2670,11 +2669,11 @@ static const char *urlsection(cmd_parms
conf->d = apr_pstrdup(cmd->pool, cmd->path); /* No mangling, please */
conf->d_is_fnmatch = apr_fnmatch_test(conf->d) != 0;
- conf->r = r;
+ conf->r = cmd->regex;
- if (r) {
+ if (cmd->regex) {
conf->refs = apr_array_make(cmd->pool, 8, sizeof(char *));
- ap_regname(r, conf->refs, AP_REG_MATCH, 1);
+ ap_regname(cmd->regex, conf->refs, AP_REG_MATCH, 1);
}
ap_add_per_url_conf(cmd->server, new_url_conf);
@@ -2697,7 +2696,6 @@ static const char *filesection(cmd_parms
int old_overrides = cmd->override;
char *old_path = cmd->path;
core_dir_config *conf;
- ap_regex_t *r = NULL;
const command_rec *thiscmd = cmd->cmd;
ap_conf_vector_t *new_file_conf = ap_create_per_dir_config(cmd->pool);
const char *err = ap_check_cmd_context(cmd,
@@ -2724,15 +2722,15 @@ static const char *filesection(cmd_parms
}
if (thiscmd->cmd_data) { /* <FilesMatch> */
- r = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED|USE_ICASE);
- if (!r) {
+ cmd->regex = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED|USE_ICASE);
+ if (!cmd->regex) {
return "Regex could not be compiled";
}
}
else if (!strcmp(cmd->path, "~")) {
cmd->path = ap_getword_conf(cmd->pool, &arg);
- r = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED|USE_ICASE);
- if (!r) {
+ cmd->regex = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED|USE_ICASE);
+ if (!cmd->regex) {
return "Regex could not be compiled";
}
}
@@ -2757,11 +2755,11 @@ static const char *filesection(cmd_parms
conf->d = cmd->path;
conf->d_is_fnmatch = apr_fnmatch_test(conf->d) != 0;
- conf->r = r;
+ conf->r = cmd->regex;
- if (r) {
+ if (cmd->regex) {
conf->refs = apr_array_make(cmd->pool, 8, sizeof(char *));
- ap_regname(r, conf->refs, AP_REG_MATCH, 1);
+ ap_regname(cmd->regex, conf->refs, AP_REG_MATCH, 1);
}
ap_add_file_conf(cmd->pool, (core_dir_config *)mconfig, new_file_conf);
Re: svn commit: r1909135 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h server/core.c
Posted by Graham Leggett via dev <de...@httpd.apache.org>.
On 19 Apr 2023, at 19:08, Yann Ylavic <yl...@gmail.com> wrote:
> Currently all the tests (framework) are broken due to cmd->regex not
> being properly stacked/scoped.
> If cmd->regex is to be used to store the enclosing section's regex, it
> should be saved and restored by *all* the the sections (much like
> cmd->override is saved/restored using the old_overrides stack variable
> in most sections). It should also be set to NULL when parsing
> (sub-)sections with no possible regex, unless we want to inherit
> cmd->regex there.
This is done in r1909356.
>
> But for instance some like:
> <Location ...>
> <If ...>
> # use /fake from <Location>
> Alias /real
> </If>
> </Location>
> won't work for Alias because <If> overwrites cmd->path with "*If".
I just found that - that’s definitely broken, it means you can’t do this:
<Location /svn/extra>
<If [something]>
DAV svn
SVNPath /home/trac/svn/extra
</If>
</Location>
Looks like the same effect as the Alias bug, where every url is mapped to “*If”.
> Also, what about:
> <DirectoryMatch ...>
> <FilesMatch ...>
> # which regex here?
> </FilesMatch>
> </DirectoryMatch>
In the above “which regex here” would be FilesMatch, which wouldn’t change.
> Or:
> <DirectoryMatch ...>
> <Files ...>
> # <DirectoryMatch>'s regex usable here?
> </Files>
> </DirectoryMatch>
> ?
Combining regexes is probably a step too far. Right now DirectoryMatch’s regex isn’t usable at the point as the DirectoryMatch regex is not passed across Files. This behaviour doesn't change.
> Or third-party sections (unaware of cmd->regex) which contain
> directives that depend on cmd->regex?
Again there would be no change to behaviour that I can see, the regex was hidden before and the raw path (containing the regex) would be used, now cmd->regex would be set to NULL and the raw path (containing the regex) would be used.
> I'm not saying it's a bad idea but it needs more changes than this
> commit, changes that spread all over the code base it seems (modules
> can have their sections too), something like:
> $ grep -r 'AP_INIT\w\+("<' server/ modules/ 2>/dev/null
> server/core.c:AP_INIT_RAW_ARGS("<Directory", dirsection, NULL, RSRC_CONF,
> server/core.c:AP_INIT_RAW_ARGS("<Location", urlsection, NULL, RSRC_CONF,
> server/core.c:AP_INIT_RAW_ARGS("<VirtualHost", virtualhost_section,
> NULL, RSRC_CONF,
[snip]
cmd->regex is a function of cmd->path, so only places that touch cmd->path need touch cmd->regex, and this seems to be directory, location and file.
<If> seems broken right now, need to fix that separately.
> Maybe we only want to check that the parent directive is a <Location>
> with Alias for now?
Right now it’s limited to not-in-directory:
https://github.com/apache/httpd/blob/trunk/modules/mappers/mod_alias.c#L146
If this is too difficult to backport it isn't the end of the world, the main thing is that it's fixed for the future.
Regards,
Graham
—
Re: svn commit: r1909135 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h server/core.c
Posted by Joe Orton <jo...@redhat.com>.
On Wed, Apr 19, 2023 at 08:08:49PM +0200, Yann Ylavic wrote:
> On Fri, Apr 14, 2023 at 4:02 PM <mi...@apache.org> wrote:
> >
> > Author: minfrin
> > Date: Fri Apr 14 14:02:11 2023
> > New Revision: 1909135
> >
> > URL: http://svn.apache.org/viewvc?rev=1909135&view=rev
> > Log:
> > core: Be explicit if an enclosing directive contains a path or a
> > regex.
>
> Currently all the tests (framework) are broken due to cmd->regex not
> being properly stacked/scoped.
Trunk has been broken for a week, Graham could you move this work to a
branch or PR until the regressions are fixed?
Regards, Joe
Re: svn commit: r1909135 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h server/core.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Apr 14, 2023 at 4:02 PM <mi...@apache.org> wrote:
>
> Author: minfrin
> Date: Fri Apr 14 14:02:11 2023
> New Revision: 1909135
>
> URL: http://svn.apache.org/viewvc?rev=1909135&view=rev
> Log:
> core: Be explicit if an enclosing directive contains a path or a
> regex.
Currently all the tests (framework) are broken due to cmd->regex not
being properly stacked/scoped.
If cmd->regex is to be used to store the enclosing section's regex, it
should be saved and restored by *all* the the sections (much like
cmd->override is saved/restored using the old_overrides stack variable
in most sections). It should also be set to NULL when parsing
(sub-)sections with no possible regex, unless we want to inherit
cmd->regex there.
But for instance some like:
<Location ...>
<If ...>
# use /fake from <Location>
Alias /real
</If>
</Location>
won't work for Alias because <If> overwrites cmd->path with "*If".
Also, what about:
<DirectoryMatch ...>
<FilesMatch ...>
# which regex here?
</FilesMatch>
</DirectoryMatch>
Or:
<DirectoryMatch ...>
<Files ...>
# <DirectoryMatch>'s regex usable here?
</Files>
</DirectoryMatch>
?
Or third-party sections (unaware of cmd->regex) which contain
directives that depend on cmd->regex?
I'm not saying it's a bad idea but it needs more changes than this
commit, changes that spread all over the code base it seems (modules
can have their sections too), something like:
$ grep -r 'AP_INIT\w\+("<' server/ modules/ 2>/dev/null
server/core.c:AP_INIT_RAW_ARGS("<Directory", dirsection, NULL, RSRC_CONF,
server/core.c:AP_INIT_RAW_ARGS("<Location", urlsection, NULL, RSRC_CONF,
server/core.c:AP_INIT_RAW_ARGS("<VirtualHost", virtualhost_section,
NULL, RSRC_CONF,
server/core.c:AP_INIT_RAW_ARGS("<Files", filesection, NULL, OR_ALL,
server/core.c:AP_INIT_RAW_ARGS("<Limit", ap_limit_section, NULL,
OR_LIMIT | OR_AUTHCFG,
server/core.c:AP_INIT_RAW_ARGS("<LimitExcept", ap_limit_section, (void*)1,
server/core.c:AP_INIT_RAW_ARGS("<IfModule", start_cond_section, (void
*)test_ifmod_section,
server/core.c:AP_INIT_RAW_ARGS("<IfDefine", start_cond_section, (void
*)test_ifdefine_section,
server/core.c:AP_INIT_RAW_ARGS("<IfFile", start_cond_section, (void
*)test_iffile_section,
server/core.c:AP_INIT_RAW_ARGS("<IfDirective", start_cond_section,
(void *)test_ifdirective_section,
server/core.c:AP_INIT_RAW_ARGS("<IfSection", start_cond_section, (void
*)test_ifsection_section,
server/core.c:AP_INIT_RAW_ARGS("<DirectoryMatch", dirsection,
(void*)1, RSRC_CONF,
server/core.c:AP_INIT_RAW_ARGS("<LocationMatch", urlsection, (void*)1,
RSRC_CONF,
server/core.c:AP_INIT_RAW_ARGS("<FilesMatch", filesection, (void*)1, OR_ALL,
server/core.c:AP_INIT_RAW_ARGS("<If", ifsection, COND_IF, OR_ALL,
server/core.c:AP_INIT_RAW_ARGS("<ElseIf", ifsection, COND_ELSEIF, OR_ALL,
server/core.c:AP_INIT_RAW_ARGS("<Else", ifsection, COND_ELSE, OR_ALL,
modules/metadata/mod_version.c: AP_INIT_TAKE123("<IfVersion",
start_ifversion, NULL, EXEC_ON_READ | OR_ALL,
modules/aaa/mod_authn_core.c:
AP_INIT_RAW_ARGS("<AuthnProviderAlias", authaliassection, NULL,
RSRC_CONF,
modules/aaa/mod_authz_core.c:
AP_INIT_RAW_ARGS("<AuthzProviderAlias", authz_require_alias_section,
modules/aaa/mod_authz_core.c: AP_INIT_RAW_ARGS("<RequireAll",
add_authz_section, NULL, OR_AUTHCFG,
modules/aaa/mod_authz_core.c: AP_INIT_RAW_ARGS("<RequireAny",
add_authz_section, NULL, OR_AUTHCFG,
modules/aaa/mod_authz_core.c: AP_INIT_RAW_ARGS("<RequireNotAll",
add_authz_section, NULL, OR_AUTHCFG,
modules/aaa/mod_authz_core.c: AP_INIT_RAW_ARGS("<RequireNone",
add_authz_section, NULL, OR_AUTHCFG,
modules/lua/mod_lua.c: AP_INIT_RAW_ARGS("<LuaHookPreTranslateName",
register_pre_trans_name_block, NULL,
modules/lua/mod_lua.c: AP_INIT_RAW_ARGS("<LuaHookTranslateName",
register_translate_name_block,
modules/lua/mod_lua.c: AP_INIT_RAW_ARGS("<LuaHookFixups",
register_fixups_block, NULL,
modules/lua/mod_lua.c: AP_INIT_RAW_ARGS("<LuaHookMapToStorage",
register_map_to_storage_block,
modules/lua/mod_lua.c: AP_INIT_RAW_ARGS("<LuaHookCheckUserID",
register_check_user_id_block,
modules/lua/mod_lua.c: AP_INIT_RAW_ARGS("<LuaHookTypeChecker",
register_type_checker_block, NULL,
modules/lua/mod_lua.c: AP_INIT_RAW_ARGS("<LuaHookAccessChecker",
register_access_checker_block,
modules/lua/mod_lua.c: AP_INIT_RAW_ARGS("<LuaHookAuthChecker",
register_auth_checker_block, NULL,
modules/lua/mod_lua.c: AP_INIT_RAW_ARGS("<LuaQuickHandler",
register_quick_block, NULL,
modules/proxy/mod_proxy.c: AP_INIT_RAW_ARGS("<Proxy", proxysection,
NULL, RSRC_CONF,
modules/proxy/mod_proxy.c: AP_INIT_RAW_ARGS("<ProxyMatch",
proxysection, (void*)1, RSRC_CONF,
(possibly not all of them need care though)
Maybe we only want to check that the parent directive is a <Location>
with Alias for now?
Regards;
Yann.