ParseXS: make most ENABLE keywords stricter

These four keywords are supposed to have only ENABLE or DISABLE
as their value:

    EXPORT_XSUB_SYMBOLS:
    SCOPE:
    VERSIONCHECK:
    PROTOTYPES:

It turns out that the XS parser is very lax and allows all sorts of
rubbish as the keyword's value.  For example all these are valid, and
the first of them was originally interpreted as *DISABLE*, due to
case-insensitive validation, but case-sensitive value interpretation:

    KEYWORD: Enable
    KEYWORD: ENABLE;
    KEYWORD: ENABLE # comment
    KEYWORD: ENABLE the quick brown fox

An earlier commit in this branch, in the course of refactoring, silently
made the value matching to be case insensitive for *all* keywords
(originally it was only CI for SCOPE).

So originally,

    PROTOTYPES: Enable

actually disabled prototypes; now it enables them. This commit and the
next will restore the original behaviour and/or make things stricter.

This commit makes all such keywords stricter, apart from PROTOTYPES,
which is much more widely used (often incorrectly) and will require more
careful backwards-compatibility handling. It's behaviour is left
untouched by this commit; the next commit will update it.

For the first three keywords, this commit makes the only acceptable
values to match be /^(ENABLE|DISABLE)\s*$/.

Author: iabyn

dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS/Node.pm

@@ -3422,14 +3422,25 @@ sub parse {
     $self->SUPER::parse($pxs); # set file/line_no, self->{text}
     my $s = $self->{text};
 
-    #XXX note that for backwards compatibility, matching DIS/ENABLE is
-    #very lax: it is case insensitive, and ignores any trailing garbage
-    # on the line
-    unless ($s =~ /^(ENABLE|DISABLE)\b/i) {
-        my ($keyword) = ($self =~ /(\w+)=/); # final component of class name
-        $pxs->death("Error: $keyword: ENABLE/DISABLE")
+    my ($keyword) = ($self =~ /(\w+)=/); # final component of class name
+
+    if ($keyword eq 'PROTOTYPES') {
+        # XXX note that for backwards compatibility, parsing the PROTOTYPES
+        # keyword's value is very lax: it is case insensitive, and ignores
+        # any trailing garbage on the line
+        unless ($s =~ /^(ENABLE|DISABLE)\b/i) {
+            my ($keyword) = ($self =~ /(\w+)=/); # final component of class name
+            $pxs->death("Error: $keyword: ENABLE/DISABLE")
+        }
+        $self->{enable} = uc($1) eq 'ENABLE' ? 1 : 0;
+    }
+    else {
+        # SCOPE / VERSIONCHECK / EXPORT_XSUB_SYMBOLS
+        $s =~ /^(ENABLE|DISABLE)\s*$/
+            or $pxs->death("Error: $keyword: ENABLE/DISABLE");
+        $self->{enable} = $1 eq 'ENABLE' ? 1 : 0;
     }
-    $self->{enable} = uc($1) eq 'ENABLE' ? 1 : 0;
+
     1;
 }
 

dist/ExtUtils-ParseXS/t/001-basic.t

@@ -4279,18 +4279,32 @@ EOF
 
     my @test_fns = (
         [
-            "VERSIONCHECK long word",
+            "VERSIONCHECK: long word",
             [ Q(<<'EOF') ],
                 |VERSIONCHECK: ENABLEblah
 EOF
             [ 1, 0, qr{Error: VERSIONCHECK: ENABLE/DISABLE}, "should die" ],
         ],
         [
-            "VERSIONCHECK trailing text (stupid but legal)",
+            "VERSIONCHECK: trailing text",
             [ Q(<<'EOF') ],
-                |VERSIONCHECK: diSAble blah # bloo +$%
+                |VERSIONCHECK: DISABLE blah # bloo +$%
 EOF
-            [ 0, 0, qr{dXSARGS}, "boot fn generated" ],
+            [ 1, 0, qr{Error: VERSIONCHECK: ENABLE/DISABLE}, "should die" ],
+        ],
+        [
+            "VERSIONCHECK: lower case",
+            [ Q(<<'EOF') ],
+                |VERSIONCHECK: disable
+EOF
+            [ 1, 0, qr{Error: VERSIONCHECK: ENABLE/DISABLE}, "should die" ],
+        ],
+        [
+            "VERSIONCHECK: semicolon",
+            [ Q(<<'EOF') ],
+                |VERSIONCHECK: DISABLE;
+EOF
+            [ 1, 0, qr{Error: VERSIONCHECK: ENABLE/DISABLE}, "should die" ],
         ],
 
         [
@@ -4316,19 +4330,35 @@ EOF
             [ 1, 0, qr{Error: EXPORT_XSUB_SYMBOLS: ENABLE/DISABLE}, "should die" ],
         ],
         [
-            "EXPORT_XSUB_SYMBOLS: trailing text (stupid but legal)",
+            "EXPORT_XSUB_SYMBOLS: trailing text",
             [ Q(<<'EOF') ],
                 |EXPORT_XSUB_SYMBOLS: diSAble blah # bloo +$%
 EOF
-            [ 0, 0, qr{dXSARGS}, "boot fn generated" ],
+            [ 1, 0, qr{Error: EXPORT_XSUB_SYMBOLS: ENABLE/DISABLE}, "should die" ],
+        ],
+        [
+            "EXPORT_XSUB_SYMBOLS: lower case",
+            [ Q(<<'EOF') ],
+                |EXPORT_XSUB_SYMBOLS: disable
+EOF
+            [ 1, 0, qr{Error: EXPORT_XSUB_SYMBOLS: ENABLE/DISABLE}, "should die" ],
         ],
 
         [
-            "file SCOPE long word",
+            "file SCOPE: long word",
             [ Q(<<'EOF') ],
                 |SCOPE: ENABLEblah
                 |void
                 |foo()
+EOF
+            [ 1, 0, qr{Error: SCOPE: ENABLE/DISABLE}, "should die" ],
+        ],
+        [
+            "file SCOPE: lower case",
+            [ Q(<<'EOF') ],
+                |SCOPE: enable
+                |void
+                |foo()
 EOF
             [ 1, 0, qr{Error: SCOPE: ENABLE/DISABLE}, "should die" ],
         ],
@@ -4358,30 +4388,46 @@ EOF
 
     my @test_fns = (
         [
-            "file SCOPE trailing text (stupid but legal)",
+            "file SCOPE: trailing text",
             [ Q(<<'EOF') ],
                 |SCOPE: EnAble blah # bloo +$%
                 |void
                 |foo()
 EOF
-            [ 0, 0, qr{ENTER;\s+{\s+\Qfoo();\E\s+}\s+LEAVE;},
-                    "has ENTER/LEAVE" ],
+            [ 1, 0, qr{Error: SCOPE: ENABLE/DISABLE}, "should die" ],
         ],
         [
-            "xsub SCOPE trailing text (stupid but legal)",
+            "xsub SCOPE: trailing text",
             [ Q(<<'EOF') ],
                 |void
                 |foo()
                 |SCOPE: EnAble blah # bloo +$%
 EOF
-            [ 0, 0, qr{ENTER;\s+{\s+\Qfoo();\E\s+}\s+LEAVE;},
-                    "has ENTER/LEAVE" ],
+            [ 1, 0, qr{Error: SCOPE: ENABLE/DISABLE}, "should die" ],
+        ],
+        [
+            "xsub SCOPE: lower case",
+            [ Q(<<'EOF') ],
+                |void
+                |foo()
+                |SCOPE: enable
+EOF
+            [ 1, 0, qr{Error: SCOPE: ENABLE/DISABLE}, "should die" ],
+        ],
+        [
+            "xsub SCOPE: semicolon",
+            [ Q(<<'EOF') ],
+                |void
+                |foo()
+                |SCOPE: ENABLE;
+EOF
+            [ 1, 0, qr{Error: SCOPE: ENABLE/DISABLE}, "should die" ],
         ],
 
         [
             "SCOPE: as file-scoped keyword",
             [ Q(<<'EOF') ],
-                |SCOPE: EnablE
+                |SCOPE: ENABLE
                 |void
                 |foo()
                 |C_ARGS: a,b,c
@@ -4395,7 +4441,7 @@ EOF
                 |void
                 |foo()
                 |C_ARGS: a,b,c
-                |SCOPE: EnablE
+                |SCOPE: ENABLE
 EOF
             [ 0, 0, qr{ENTER;\s+{\s+\Qfoo(a,b,c);\E\s+}\s+LEAVE;},
                     "has ENTER/LEAVE" ],