You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2019/06/18 05:56:01 UTC
Re: svn commit: r1861542 - in /httpd/httpd/trunk:
docs/manual/mod/mod_alias.xml modules/mappers/mod_alias.c
On 06/17/2019 08:35 PM, covener@apache.org wrote:
> Author: covener
> Date: Mon Jun 17 18:35:24 2019
> New Revision: 1861542
>
> URL: http://svn.apache.org/viewvc?rev=1861542&view=rev
> Log:
> add RedirectRelative directive to allow relative Redirect targets
>
> 2616 forbade relative redirect URLs, but 7231 allows them
> Early 2.2 maintenance levels did not fix them up, but later 2.2 and all 2.4
> fixed them up with ap_construct_url().
>
> Allow opt-in to not fixing up relative URLs with RedirectRelative
>
>
>
> Modified:
> httpd/httpd/trunk/docs/manual/mod/mod_alias.xml
> httpd/httpd/trunk/modules/mappers/mod_alias.c
>
> Modified: httpd/httpd/trunk/modules/mappers/mod_alias.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_alias.c?rev=1861542&r1=1861541&r2=1861542&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/mappers/mod_alias.c (original)
> +++ httpd/httpd/trunk/modules/mappers/mod_alias.c Mon Jun 17 18:35:24 2019
> @@ -37,6 +37,10 @@
> #include "ap_expr.h"
>
>
> +#define ALIAS_FLAG_DEFAULT -1
> +#define ALIAS_FLAG_OFF 0
> +#define ALIAS_FLAG_ON 1
> +
> typedef struct {
> const char *real;
> const char *fake;
> @@ -58,6 +62,7 @@ typedef struct {
> char *handler;
> const ap_expr_info_t *redirect;
> int redirect_status; /* 301, 302, 303, 410, etc */
> + int allow_relative; /* skip ap_construct_url() */
> } alias_dir_conf;
>
> module AP_MODULE_DECLARE_DATA alias_module;
> @@ -80,6 +85,7 @@ static void *create_alias_dir_config(apr
> alias_dir_conf *a =
> (alias_dir_conf *) apr_pcalloc(p, sizeof(alias_dir_conf));
> a->redirects = apr_array_make(p, 2, sizeof(alias_entry));
> + a->allow_relative = ALIAS_FLAG_DEFAULT;
> return a;
> }
>
> @@ -111,7 +117,9 @@ static void *merge_alias_dir_config(apr_
> a->redirect = (overrides->redirect_set == 0) ? base->redirect : overrides->redirect;
> a->redirect_status = (overrides->redirect_set == 0) ? base->redirect_status : overrides->redirect_status;
> a->redirect_set = overrides->redirect_set || base->redirect_set;
> -
> + a->allow_relative = (overrides->allow_relative != ALIAS_FLAG_DEFAULT)
> + ? overrides->allow_relative
> + : base->allow_relative;
> return a;
> }
>
> @@ -591,31 +599,33 @@ static int translate_alias_redir(request
> if (ret == PREGSUB_ERROR)
> return HTTP_INTERNAL_SERVER_ERROR;
> if (ap_is_HTTP_REDIRECT(status)) {
> - if (ret[0] == '/') {
> - char *orig_target = ret;
> -
> - ret = ap_construct_url(r->pool, ret, r);
> - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00673)
> - "incomplete redirection target of '%s' for "
> - "URI '%s' modified to '%s'",
> - orig_target, r->uri, ret);
> - }
> - if (!ap_is_url(ret)) {
> - status = HTTP_INTERNAL_SERVER_ERROR;
> - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00674)
> - "cannot redirect '%s' to '%s'; "
> - "target is not a valid absoluteURI or abs_path",
> - r->uri, ret);
> - }
> - else {
> - /* append requested query only, if the config didn't
> - * supply its own.
> - */
> - if (r->args && !ap_strchr(ret, '?')) {
> - ret = apr_pstrcat(r->pool, ret, "?", r->args, NULL);
> + alias_dir_conf *dirconf = (alias_dir_conf *)
> + ap_get_module_config(r->per_dir_config, &alias_module);
> + if (dirconf->allow_relative != ALIAS_FLAG_ON || ret[0] != '/') {
> + if (ret[0] == '/') {
> + char *orig_target = ret;
> +
> + ret = ap_construct_url(r->pool, ret, r);
> + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00673)
> + "incomplete redirection target of '%s' for "
> + "URI '%s' modified to '%s'",
> + orig_target, r->uri, ret);
> }
> - apr_table_setn(r->headers_out, "Location", ret);
> + if (!ap_is_url(ret)) {
> + status = HTTP_INTERNAL_SERVER_ERROR;
> + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00674)
> + "cannot redirect '%s' to '%s'; "
> + "target is not a valid absoluteURI or abs_path",
> + r->uri, ret);
In the old code path we did not set the Location header and did not append the args if
apr_is_url fails. So shouldn't we do a
return status
here in the block?
> + }
> + }
> + /* append requested query only, if the config didn't
> + * supply its own.
> + */
> + if (r->args && !ap_strchr(ret, '?')) {
> + ret = apr_pstrcat(r->pool, ret, "?", r->args, NULL);
> }
> + apr_table_setn(r->headers_out, "Location", ret);
> }
> return status;
> }
> @@ -646,31 +656,31 @@ static int fixup_redir(request_rec *r)
> if (ret == PREGSUB_ERROR)
> return HTTP_INTERNAL_SERVER_ERROR;
> if (ap_is_HTTP_REDIRECT(status)) {
> - if (ret[0] == '/') {
> - char *orig_target = ret;
> -
> - ret = ap_construct_url(r->pool, ret, r);
> - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00675)
> - "incomplete redirection target of '%s' for "
> - "URI '%s' modified to '%s'",
> - orig_target, r->uri, ret);
> - }
> - if (!ap_is_url(ret)) {
> - status = HTTP_INTERNAL_SERVER_ERROR;
> - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00676)
> - "cannot redirect '%s' to '%s'; "
> - "target is not a valid absoluteURI or abs_path",
> - r->uri, ret);
> - }
> - else {
> - /* append requested query only, if the config didn't
> - * supply its own.
> - */
> - if (r->args && !ap_strchr(ret, '?')) {
> - ret = apr_pstrcat(r->pool, ret, "?", r->args, NULL);
> + if (dirconf->allow_relative != ALIAS_FLAG_ON || ret[0] != '/') {
> + if (ret[0] == '/') {
> + char *orig_target = ret;
> +
> + ret = ap_construct_url(r->pool, ret, r);
> + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00675)
> + "incomplete redirection target of '%s' for "
> + "URI '%s' modified to '%s'",
> + orig_target, r->uri, ret);
> + }
> + if (!ap_is_url(ret)) {
> + status = HTTP_INTERNAL_SERVER_ERROR;
> + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00676)
> + "cannot redirect '%s' to '%s'; "
> + "target is not a valid absoluteURI or abs_path",
> + r->uri, ret);
Same comment as above.
> }
> - apr_table_setn(r->headers_out, "Location", ret);
> }
> + /* append requested query only, if the config didn't
> + * supply its own.
> + */
> + if (r->args && !ap_strchr(ret, '?')) {
> + ret = apr_pstrcat(r->pool, ret, "?", r->args, NULL);
> + }
> + apr_table_setn(r->headers_out, "Location", ret);
> }
> return status;
> }
Regards
Rüdiger
Re: svn commit: r1861542 - in /httpd/httpd/trunk: docs/manual/mod/mod_alias.xml
modules/mappers/mod_alias.c
Posted by Eric Covener <co...@gmail.com>.
> In the old code path we did not set the Location header and did not append the args if
> apr_is_url fails. So shouldn't we do a
Thanks, r1861569