You are viewing a plain text version of this content. The canonical link for it is here.
Posted to bugs@httpd.apache.org by bu...@apache.org on 2012/04/02 10:18:00 UTC

DO NOT REPLY [Bug 53023] New: mod_rewrite RewriteCond parser error and documentation inconsistency

https://issues.apache.org/bugzilla/show_bug.cgi?id=53023

             Bug #: 53023
           Summary: mod_rewrite RewriteCond parser error and documentation
                    inconsistency
           Product: Apache httpd-2
           Version: 2.4.1
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: mod_rewrite
        AssignedTo: bugs@httpd.apache.org
        ReportedBy: apache@freakout.de
    Classification: Unclassified


mod_rewrite additional syntax to perform other useful tests against
Teststring variant 3 (->"You can perform integer comparisons") is
completely broken because the parser never recognizes the options
-eq, -ge, ... because the 2nd argument is tested to have only two
characters (-f, -L, ...) so these other options are never seen by
the code below which checks for these options - see my comments
in the code below.

In contrast to variant 2 (->"You can perform lexicographical
string comparisons") where the CondPattern is explicitely mentioned
in the syntax description, the documentation of variant 3 does not
describe how to apply the CondPattern.

It is unclear how the CondPattern is going into the RewriteCond command,
there is also nowhere an example for variant 3.

    else if (*a2 && a2[1]) {
        if (!a2[2] && *a2 == '-') { /* !!! test string to only two chars */
            switch (a2[1]) {
            case 'f': newcond->ptype = CONDPAT_FILE_EXISTS; break;
            case 's': newcond->ptype = CONDPAT_FILE_SIZE;   break;
            case 'd': newcond->ptype = CONDPAT_FILE_DIR;    break;
            case 'x': newcond->ptype = CONDPAT_FILE_XBIT;   break;
            case 'h': newcond->ptype = CONDPAT_FILE_LINK;   break;
            case 'L': newcond->ptype = CONDPAT_FILE_LINK;   break;
            case 'U': newcond->ptype = CONDPAT_LU_URL;      break;
            case 'F': newcond->ptype = CONDPAT_LU_FILE;     break;
            /* !!! these cases can never be reached due to the two-char test
above */
            case 'l': if (a2[2] == 't')
                          a2 += 3, newcond->ptype = CONDPAT_INT_LT;
                      else if (a2[2] == 'e')
                          a2 += 3, newcond->ptype = CONDPAT_INT_LE;
                      else /* Historical; prefer -L or -h instead */
                          newcond->ptype = CONDPAT_FILE_LINK;
                      break;
            case 'g': if (a2[2] == 't')
                          a2 += 3, newcond->ptype = CONDPAT_INT_GT;
                      else if (a2[2] == 'e')
                          a2 += 3, newcond->ptype = CONDPAT_INT_GE;
                      break;
            case 'e': if (a2[2] == 'q')
                          a2 += 3, newcond->ptype = CONDPAT_INT_EQ;
                      break;
            case 'n': if (a2[2] == 'e') {
                          /* Inversion, ensure !-ne == -eq */
                          a2 += 3, newcond->ptype = CONDPAT_INT_EQ;
                          newcond->flags ^= CONDFLAG_NOTMATCH;
                      }
                      break;
            }
        }

for me the following patch fixes the defect for me:

--- modules/mappers/mod_rewrite.c       Thu Jan 26 20:15:41 2012
+++ modules/mappers/mod_rewrite.c       Mon Apr  2 09:04:12 2012
@@ -3243,7 +3243,7 @@
         newcond->ptype = CONDPAT_AP_EXPR;
     }
     else if (*a2 && a2[1]) {
-        if (!a2[2] && *a2 == '-') {
+        if (*a2 == '-') {
             switch (a2[1]) {
             case 'f': newcond->ptype = CONDPAT_FILE_EXISTS; break;
             case 's': newcond->ptype = CONDPAT_FILE_SIZE;   break;

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


DO NOT REPLY [Bug 53023] mod_rewrite RewriteCond parser error and documentation inconsistency

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53023

--- Comment #1 from André Malo <nd...@perlig.de> 2012-04-02 18:23:17 UTC ---
Hmm, the patch doesn't look right. It seems to allow things like -foo

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 53023] mod_rewrite RewriteCond parser error and documentation inconsistency

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53023

--- Comment #8 from Axel Reinhold <ap...@freakout.de> ---
the proposed patch break the historical but still documented and used -l usage.
this new patch fixes the -lt and left the -l usable:
--- modules/mappers/mod_rewrite.c       Thu Jan 26 20:15:41 2012
+++ modules/mappers/mod_rewrite.c       Fri May  4 14:01:13 2012
@@ -3249,16 +3249,19 @@
             case 's': newcond->ptype = CONDPAT_FILE_SIZE;   break;
             case 'd': newcond->ptype = CONDPAT_FILE_DIR;    break;
             case 'x': newcond->ptype = CONDPAT_FILE_XBIT;   break;
+            case 'l': newcond->ptype = CONDPAT_FILE_LINK;   break;
             case 'h': newcond->ptype = CONDPAT_FILE_LINK;   break;
             case 'L': newcond->ptype = CONDPAT_FILE_LINK;   break;
             case 'U': newcond->ptype = CONDPAT_LU_URL;      break;
             case 'F': newcond->ptype = CONDPAT_LU_FILE;     break;
+            }
+        }
+        else if (*a2 == '-') {
+            switch (a2[1]) {
             case 'l': if (a2[2] == 't')
                           a2 += 3, newcond->ptype = CONDPAT_INT_LT;
                       else if (a2[2] == 'e')
                           a2 += 3, newcond->ptype = CONDPAT_INT_LE;
-                      else /* Historical; prefer -L or -h instead */
-                          newcond->ptype = CONDPAT_FILE_LINK;
                       break;
             case 'g': if (a2[2] == 't')
                           a2 += 3, newcond->ptype = CONDPAT_INT_GT;

-- 
You are receiving this mail because:
You are the assignee for the bug.

DO NOT REPLY [Bug 53023] mod_rewrite RewriteCond parser error and documentation inconsistency

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53023

--- Comment #2 from Axel Reinhold <ap...@freakout.de> 2012-04-02 19:55:05 UTC ---
no it does not "allow" '-foo' it just pass '-foo' the same way as without the
patch to the rest of the routine. the patch just allows the longer options to
get recognized by the parser. curious - it seems that nobody ever used the
variant 3 of the other useful tests. almost unbelievable.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


DO NOT REPLY [Bug 53023] mod_rewrite RewriteCond parser error and documentation inconsistency

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53023

--- Comment #4 from Axel Reinhold <ap...@freakout.de> 2012-04-03 05:05:18 UTC ---
but that would be the same without my patch - it would be "allowed" the same
way. the -gt part could never get a single test, because the option was never
parsed.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


DO NOT REPLY [Bug 53023] mod_rewrite RewriteCond parser error and documentation inconsistency

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53023

--- Comment #3 from André Malo <nd...@perlig.de> 2012-04-02 20:54:22 UTC ---
Hmm, yes, it *does* allow -foo:

    RewriteEngine on
    RewriteCond /xxx -foo
    RewriteRule ^ - [R=503]

put that into a virtualhost, touch /xxx and get a 503. Just tested (with 2.2
though)

I agree, that the whole piece of code is badly tested, though (the part with
-gt, which is new)

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


DO NOT REPLY [Bug 53023] mod_rewrite RewriteCond parser error and documentation inconsistency

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53023

--- Comment #6 from Axel Reinhold <ap...@freakout.de> 2012-04-03 08:22:04 UTC ---
You are right - all the -Xfoo fail when X is one of the allowed one-letter
options. Next try:

--- modules/mappers/mod_rewrite.c.orig        Thu Jan 26 20:15:41 2012
+++ modules/mappers/mod_rewrite.c     Tue Apr  3 10:16:01 2012
@@ -3253,6 +3253,10 @@
             case 'L': newcond->ptype = CONDPAT_FILE_LINK;   break;
             case 'U': newcond->ptype = CONDPAT_LU_URL;      break;
             case 'F': newcond->ptype = CONDPAT_LU_FILE;     break;
+            }
+        }
+        else if (*a2 == '-') {
+            switch (a2[1]) {
             case 'l': if (a2[2] == 't')
                           a2 += 3, newcond->ptype = CONDPAT_INT_LT;
                       else if (a2[2] == 'e')

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


DO NOT REPLY [Bug 53023] mod_rewrite RewriteCond parser error and documentation inconsistency

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53023

--- Comment #7 from André Malo <nd...@perlig.de> 2012-04-04 21:29:14 UTC ---
Submitted (modified) in trunk
(<http://svn.apache.org/viewvc?view=revision&revision=1309602>) and proposed
for backport into 2.4.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


DO NOT REPLY [Bug 53023] mod_rewrite RewriteCond parser error and documentation inconsistency

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53023

--- Comment #5 from André Malo <nd...@perlig.de> 2012-04-03 07:57:16 UTC ---
No, because the !a2[2] test forbids it. It checks if there's nothing (i.e. \0)
after the -f. Without the !a2[2] test, everything after "-f" (or another tested
letter) is suddenly allowed.
The two-letter-checks need to go into a different if-branch. I did not check,
but maybe they even were at some point and someone moved code around without
testing.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org