Re: svn commit: r1918651 - in /httpd/httpd/trunk: include/ap_mmn.h include/httpd.h modules/mappers/mod_rewrite.c server/util.c

2024-06-26 Thread Eric Covener
On Wed, Jun 26, 2024 at 9:30 AM Ruediger Pluem  wrote:
>
>
>
> On 6/26/24 12:09 PM, [email protected] wrote:
> > Author: covener
> > Date: Wed Jun 26 10:09:29 2024
> > New Revision: 1918651
> >
> > URL: http://svn.apache.org/viewvc?rev=1918651&view=rev
> > Log:
> > factor out IS_SLASH, perdir fix
> >
> > in per-dir, the filename will be internally redirected, so / is OK too.
> >
> > Modified:
> > httpd/httpd/trunk/include/ap_mmn.h
> > httpd/httpd/trunk/include/httpd.h
> > httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> > httpd/httpd/trunk/server/util.c
> >
>
> > Modified: httpd/httpd/trunk/include/httpd.h
> > URL: 
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/include/httpd.h?rev=1918651&r1=1918650&r2=1918651&view=diff
> > ==
> > --- httpd/httpd/trunk/include/httpd.h (original)
> > +++ httpd/httpd/trunk/include/httpd.h Wed Jun 26 10:09:29 2024
> > @@ -2879,6 +2879,17 @@ AP_DECLARE(apr_status_t) ap_filepath_mer
> >  #define apr_filepath_merge  ap_filepath_merge
> >  #endif
> >
> > +/* Win32/NetWare/OS2 need to check for both forward and back slashes
> > + * in ap_normalize_path() and ap_escape_url().
> > + */
> > +#ifdef CASE_BLIND_FILESYSTEM
> > +#define AP_IS_SLASH(s) ((s == '/') || (s == '\\'))
> > +#define AP_SLASHES "/\\"
> > +#else
> > +#define AP_IS_SLASH(s) (s == '/')
> > +#define AP_SLASHES "/"
> > +#endif
> > +
> >  #ifdef __cplusplus
> >  }
> >  #endif
> >
> > Modified: httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> > URL: 
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_rewrite.c?rev=1918651&r1=1918650&r2=1918651&view=diff
> > ==
> > --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original)
> > +++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Wed Jun 26 10:09:29 2024
> > @@ -688,11 +688,8 @@ static int is_absolute_path(const char *
> >  #ifndef WIN32
>
> Don't we need to change the above to
>
> #ifndef CASE_BLIND_FILESYSTEM

ty, matched them up in r1918663


Re: svn commit: r1918651 - in /httpd/httpd/trunk: include/ap_mmn.h include/httpd.h modules/mappers/mod_rewrite.c server/util.c

2024-06-26 Thread Ruediger Pluem



On 6/26/24 12:09 PM, [email protected] wrote:
> Author: covener
> Date: Wed Jun 26 10:09:29 2024
> New Revision: 1918651
> 
> URL: http://svn.apache.org/viewvc?rev=1918651&view=rev
> Log:
> factor out IS_SLASH, perdir fix
> 
> in per-dir, the filename will be internally redirected, so / is OK too.
> 
> Modified:
> httpd/httpd/trunk/include/ap_mmn.h
> httpd/httpd/trunk/include/httpd.h
> httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> httpd/httpd/trunk/server/util.c
> 

> Modified: httpd/httpd/trunk/include/httpd.h
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/include/httpd.h?rev=1918651&r1=1918650&r2=1918651&view=diff
> ==
> --- httpd/httpd/trunk/include/httpd.h (original)
> +++ httpd/httpd/trunk/include/httpd.h Wed Jun 26 10:09:29 2024
> @@ -2879,6 +2879,17 @@ AP_DECLARE(apr_status_t) ap_filepath_mer
>  #define apr_filepath_merge  ap_filepath_merge
>  #endif
>  
> +/* Win32/NetWare/OS2 need to check for both forward and back slashes
> + * in ap_normalize_path() and ap_escape_url().
> + */
> +#ifdef CASE_BLIND_FILESYSTEM
> +#define AP_IS_SLASH(s) ((s == '/') || (s == '\\'))
> +#define AP_SLASHES "/\\"
> +#else
> +#define AP_IS_SLASH(s) (s == '/')
> +#define AP_SLASHES "/"
> +#endif
> +
>  #ifdef __cplusplus
>  }
>  #endif
> 
> Modified: httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_rewrite.c?rev=1918651&r1=1918650&r2=1918651&view=diff
> ==
> --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original)
> +++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Wed Jun 26 10:09:29 2024
> @@ -688,11 +688,8 @@ static int is_absolute_path(const char *
>  #ifndef WIN32

Don't we need to change the above to

#ifndef CASE_BLIND_FILESYSTEM

or if not shouldn't we do

return AP_IS_SLASH(path[0])

>  return (path[0] == '/');
>  #else
> -#define IS_SLASH(c) ((c) == '/' || (c) == '\\')
> -/* "//", "\\", "x:/" and "x:\" are absolute paths on Windows */
> -return ((IS_SLASH(path[0]) && path[1] == path[0])
> -|| (apr_isalpha(path[0]) && path[1] == ':' && 
> IS_SLASH(path[2])));
> -#undef IS_SLASH
> +return ((AP_IS_SLASH(path[0]) && path[1] == path[0])
> +|| (apr_isalpha(path[0]) && path[1] == ':' && 
> AP_IS_SLASH(path[2])));
>  #endif
>  }
>  

Regards

Rüdiger



Re: svn commit: r1918651 - in /httpd/httpd/trunk: include/ap_mmn.h include/httpd.h modules/mappers/mod_rewrite.c server/util.c

2024-06-26 Thread Eric Covener
On Wed, Jun 26, 2024 at 6:20 AM Yann Ylavic  wrote:
>
> On Wed, Jun 26, 2024 at 12:09 PM  wrote:
> >
> > --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original)
> > +++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Wed Jun 26 10:09:29 2024
> > @@ -4413,10 +4410,15 @@ static rule_return_type apply_rewrite_ru
> >  && !is_absolute_path(newuri)
> >  && !is_absolute_uri(newuri, NULL)) {
> >  if (ctx->perdir) {
> > -rewritelog(r, 3, ctx->perdir, "add per-dir prefix: %s -> %s%s",
> > -   newuri, ctx->perdir, newuri);
> > +if (!AP_IS_SLASH(*newuri)) {
> > +/* perdir, the newuri will be internally redirected, so
> > + * leading slash is enough even if it's an ambiguous fs 
> > path
> > + */
> > +rewritelog(r, 3, ctx->perdir, "add per-dir prefix: %s -> 
> > %s%s",
> > +   newuri, ctx->perdir, newuri);
> >
> > -newuri = apr_pstrcat(r->pool, ctx->perdir, newuri, NULL);
> > +newuri = apr_pstrcat(r->pool, ctx->perdir, newuri, NULL);
> > +}
> >  }
> >  else if (!(p->flags & (RULEFLAG_PROXY | RULEFLAG_FORCEREDIRECT))) {
> >  /* Not an absolute URI-path and the scheme (if any) is unknown,
>
> Maybe the test for !AP_IS_SLASH() could be moved up (i.e.
> !AP_IS_SLASH() && !is_absolute_path()) if we don't want to add a '/'
> before anything Windows considers a slash too?

I initially had it, then didn't,
But looking again, I guess it is futile to prefix /foo with / anyway
and that's all it's gating.


Re: svn commit: r1918651 - in /httpd/httpd/trunk: include/ap_mmn.h include/httpd.h modules/mappers/mod_rewrite.c server/util.c

2024-06-26 Thread Yann Ylavic
On Wed, Jun 26, 2024 at 12:09 PM  wrote:
>
> --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original)
> +++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Wed Jun 26 10:09:29 2024
> @@ -4413,10 +4410,15 @@ static rule_return_type apply_rewrite_ru
>  && !is_absolute_path(newuri)
>  && !is_absolute_uri(newuri, NULL)) {
>  if (ctx->perdir) {
> -rewritelog(r, 3, ctx->perdir, "add per-dir prefix: %s -> %s%s",
> -   newuri, ctx->perdir, newuri);
> +if (!AP_IS_SLASH(*newuri)) {
> +/* perdir, the newuri will be internally redirected, so
> + * leading slash is enough even if it's an ambiguous fs path
> + */
> +rewritelog(r, 3, ctx->perdir, "add per-dir prefix: %s -> 
> %s%s",
> +   newuri, ctx->perdir, newuri);
>
> -newuri = apr_pstrcat(r->pool, ctx->perdir, newuri, NULL);
> +newuri = apr_pstrcat(r->pool, ctx->perdir, newuri, NULL);
> +}
>  }
>  else if (!(p->flags & (RULEFLAG_PROXY | RULEFLAG_FORCEREDIRECT))) {
>  /* Not an absolute URI-path and the scheme (if any) is unknown,

Maybe the test for !AP_IS_SLASH() could be moved up (i.e.
!AP_IS_SLASH() && !is_absolute_path()) if we don't want to add a '/'
before anything Windows considers a slash too?


Regards;
Yann.