You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by wr...@apache.org on 2016/07/29 15:35:57 UTC

svn commit: r1754538 - /httpd/httpd/trunk/server/gen_test_char.c

Author: wrowe
Date: Fri Jul 29 15:35:56 2016
New Revision: 1754538

URL: http://svn.apache.org/viewvc?rev=1754538&view=rev
Log:
Correct T_HTTP_TOKEN_STOP per RFC2068 (2.2) - RFC7230 (3.2.6),
which has always defined 'token' as CHAR or VCHAR - visible USASCII only.

NUL char is also a stop, end of parsing.


Modified:
    httpd/httpd/trunk/server/gen_test_char.c

Modified: httpd/httpd/trunk/server/gen_test_char.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/gen_test_char.c?rev=1754538&r1=1754537&r2=1754538&view=diff
==============================================================================
--- httpd/httpd/trunk/server/gen_test_char.c (original)
+++ httpd/httpd/trunk/server/gen_test_char.c Fri Jul 29 15:35:56 2016
@@ -115,8 +115,13 @@ int main(int argc, char *argv[])
             flags |= T_ESCAPE_URLENCODED;
         }
 
-        /* these are the "tspecials" (RFC2068) or "separators" (RFC2616) */
-        if (c && (apr_iscntrl(c) || strchr(" \t()<>@,;:\\\"/[]?={}", c))) {
+        /* Stop for any non-'token' character, including ctrls, obs-text,
+         * and "tspecials" (RFC2068) a.k.a. "separators" (RFC2616)
+         * XXX: With luck, isascii behaves sensibly on EBCDIC platforms
+         *      and insists on chars that correspond to ASCII equivilants
+         */
+        if (apr_iscntrl(c) || strchr(" \t()<>@,;:\\\"/[]?={}", c))
+                           || !apr_isascii(c)) {
             flags |= T_HTTP_TOKEN_STOP;
         }
 



Re: svn commit: r1754538 - /httpd/httpd/trunk/server/gen_test_char.c

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Fri, Jul 29, 2016 at 1:00 PM, Eric Covener <co...@gmail.com> wrote:

>
> Sounds reasonable, given as you proposed it doesn't implicitly get
> used in any existing portability stuff.  But hard to get excited about
> pushing it down if we end up adding it in three places.
>

Thinking of something this simple...

Index: include/apr_lib.h
===================================================================
--- include/apr_lib.h (revision 1754406)
+++ include/apr_lib.h (working copy)
@@ -217,6 +217,14 @@
 #else
 #define apr_isascii(c) (((c) & ~0x7f)==0)
 #endif
+#if APR_CHARSET_EBCDIC
+APR_DECLARE(int) apr_isascii_equiv_fn(int c);
+#define apr_isascii_equiv(c) (apr_isascii_equiv_fn(((unsigned char)(c))))
+#elif defined(isascii)
+#define apr_isascii_equiv(c) (isascii(((unsigned char)(c))))
+#else
+#define apr_isascii_equiv(c) (((c) & ~0x7f)==0)
+#endif
 /** @see isprint */
 #define apr_isprint(c) (isprint(((unsigned char)(c))))
 /** @see ispunct */
Index: strings/apr_cstr.c
===================================================================
--- strings/apr_cstr.c (revision 1754406)
+++ strings/apr_cstr.c (working copy)
@@ -279,6 +279,12 @@
 };
 #endif

+#if APR_CHARSET_EBCDIC
+int apr_isascii_equiv_fn(int c) {
+    return ((ucharmap[(unsigned char)c] & ~0x7f) == 0);
+}
+#endif
+
 int apr_cstr_casecmp(const char *s1, const char *s2)
 {
     const unsigned char *str1 = (const unsigned char *)s1;


And as you say... annoying that it ends up replicated a few times,
but that is what it is until httpd 2.next/3.next.

Re: svn commit: r1754538 - /httpd/httpd/trunk/server/gen_test_char.c

Posted by Eric Covener <co...@gmail.com>.
On Fri, Jul 29, 2016 at 1:44 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> On Fri, Jul 29, 2016 at 11:51 AM, Eric Covener <co...@gmail.com> wrote:
>>
>> On Fri, Jul 29, 2016 at 12:45 PM, William A Rowe Jr <wr...@rowe-clan.net>
>> wrote:
>> > Borrow our mapping table created for ap_cstr_tolower?
>> yep, even simpler.
>
>
> What's your thought on adding apr_isascii_equiv() as an identity to isascii
> on most platforms, and as a function of our mapping table on EBCDIC?
>
> Of course we'll need an ap_ flavor in the interim, and we can't directly
> consume that from gen_test_char.c without some trickery.

Sounds reasonable, given as you proposed it doesn't implicitly get
used in any existing portability stuff.  But hard to get excited about
pushing it down if we end up adding it in three places.

Re: svn commit: r1754538 - /httpd/httpd/trunk/server/gen_test_char.c

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Fri, Jul 29, 2016 at 11:51 AM, Eric Covener <co...@gmail.com> wrote:

> On Fri, Jul 29, 2016 at 12:45 PM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> > Borrow our mapping table created for ap_cstr_tolower?
> yep, even simpler.
>

What's your thought on adding apr_isascii_equiv() as an identity to isascii
on most platforms, and as a function of our mapping table on EBCDIC?

Of course we'll need an ap_ flavor in the interim, and we can't directly
consume that from gen_test_char.c without some trickery.

Re: svn commit: r1754538 - /httpd/httpd/trunk/server/gen_test_char.c

Posted by Eric Covener <co...@gmail.com>.
On Fri, Jul 29, 2016 at 12:45 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> Borrow our mapping table created for ap_cstr_tolower?
yep, even simpler.

Re: svn commit: r1754538 - /httpd/httpd/trunk/server/gen_test_char.c

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
Yea, that isn't going to obtain the desired effect.

Borrow our mapping table created for ap_cstr_tolower?

On Jul 29, 2016 11:25 AM, "Eric Covener" <co...@gmail.com> wrote:

> AFAICT On z/OS, with default compiler flags for "cc" (=_XOPEN_SOURCE),
> isascii() is the same as the APR fallback for isascii()
> #define apr_isascii(c) (((c) & ~0x7f)==0).
>
> Since these functions usually look at EBCDIC strings, I don't think it
> does the right thing here by itself. But since it's just gen_test_char
> a small static could
> do apr_xlate_conv_byte(ap_hdrs_to_ascii...) then isascci to see if it
> ended up -1, 7bit, or 8bit?
>
>
> On Fri, Jul 29, 2016 at 11:35 AM,  <wr...@apache.org> wrote:
> > Author: wrowe
> > Date: Fri Jul 29 15:35:56 2016
> > New Revision: 1754538
> >
> > URL: http://svn.apache.org/viewvc?rev=1754538&view=rev
> > Log:
> > Correct T_HTTP_TOKEN_STOP per RFC2068 (2.2) - RFC7230 (3.2.6),
> > which has always defined 'token' as CHAR or VCHAR - visible USASCII only.
> >
> > NUL char is also a stop, end of parsing.
> >
> >
> > Modified:
> >     httpd/httpd/trunk/server/gen_test_char.c
> >
> > Modified: httpd/httpd/trunk/server/gen_test_char.c
> > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/gen_test_char.c?rev=1754538&r1=1754537&r2=1754538&view=diff
> >
> ==============================================================================
> > --- httpd/httpd/trunk/server/gen_test_char.c (original)
> > +++ httpd/httpd/trunk/server/gen_test_char.c Fri Jul 29 15:35:56 2016
> > @@ -115,8 +115,13 @@ int main(int argc, char *argv[])
> >              flags |= T_ESCAPE_URLENCODED;
> >          }
> >
> > -        /* these are the "tspecials" (RFC2068) or "separators"
> (RFC2616) */
> > -        if (c && (apr_iscntrl(c) || strchr(" \t()<>@,;:\\\"/[]?={}",
> c))) {
> > +        /* Stop for any non-'token' character, including ctrls,
> obs-text,
> > +         * and "tspecials" (RFC2068) a.k.a. "separators" (RFC2616)
> > +         * XXX: With luck, isascii behaves sensibly on EBCDIC platforms
> > +         *      and insists on chars that correspond to ASCII
> equivilants
> > +         */
> > +        if (apr_iscntrl(c) || strchr(" \t()<>@,;:\\\"/[]?={}", c))
> > +                           || !apr_isascii(c)) {
> >              flags |= T_HTTP_TOKEN_STOP;
> >          }
> >
> >
> >
>
>
>
> --
> Eric Covener
> covener@gmail.com
>

Re: svn commit: r1754538 - /httpd/httpd/trunk/server/gen_test_char.c

Posted by Eric Covener <co...@gmail.com>.
AFAICT On z/OS, with default compiler flags for "cc" (=_XOPEN_SOURCE),
isascii() is the same as the APR fallback for isascii()
#define apr_isascii(c) (((c) & ~0x7f)==0).

Since these functions usually look at EBCDIC strings, I don't think it
does the right thing here by itself. But since it's just gen_test_char
a small static could
do apr_xlate_conv_byte(ap_hdrs_to_ascii...) then isascci to see if it
ended up -1, 7bit, or 8bit?


On Fri, Jul 29, 2016 at 11:35 AM,  <wr...@apache.org> wrote:
> Author: wrowe
> Date: Fri Jul 29 15:35:56 2016
> New Revision: 1754538
>
> URL: http://svn.apache.org/viewvc?rev=1754538&view=rev
> Log:
> Correct T_HTTP_TOKEN_STOP per RFC2068 (2.2) - RFC7230 (3.2.6),
> which has always defined 'token' as CHAR or VCHAR - visible USASCII only.
>
> NUL char is also a stop, end of parsing.
>
>
> Modified:
>     httpd/httpd/trunk/server/gen_test_char.c
>
> Modified: httpd/httpd/trunk/server/gen_test_char.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/gen_test_char.c?rev=1754538&r1=1754537&r2=1754538&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/gen_test_char.c (original)
> +++ httpd/httpd/trunk/server/gen_test_char.c Fri Jul 29 15:35:56 2016
> @@ -115,8 +115,13 @@ int main(int argc, char *argv[])
>              flags |= T_ESCAPE_URLENCODED;
>          }
>
> -        /* these are the "tspecials" (RFC2068) or "separators" (RFC2616) */
> -        if (c && (apr_iscntrl(c) || strchr(" \t()<>@,;:\\\"/[]?={}", c))) {
> +        /* Stop for any non-'token' character, including ctrls, obs-text,
> +         * and "tspecials" (RFC2068) a.k.a. "separators" (RFC2616)
> +         * XXX: With luck, isascii behaves sensibly on EBCDIC platforms
> +         *      and insists on chars that correspond to ASCII equivilants
> +         */
> +        if (apr_iscntrl(c) || strchr(" \t()<>@,;:\\\"/[]?={}", c))
> +                           || !apr_isascii(c)) {
>              flags |= T_HTTP_TOKEN_STOP;
>          }
>
>
>



-- 
Eric Covener
covener@gmail.com