Merge pull request #484 from DinoChiesa/issue483-CC005

fix: issue483 CC005 no longer modifies truthtable
apigee · Nov 1, 2024 · 6cdd5f8 · 6cdd5f8
2 parents 4e64b92 + 9812af5
commit 6cdd5f8
Show file tree

Hide file tree

Showing 32 changed files with 851 additions and 84 deletions.
diff --git a/lib/package/plugins/CC005-unterminatedStrings.js b/lib/package/plugins/CC005-unterminatedStrings.js
@@ -16,90 +16,93 @@
 
 // CC005-unterminatedStrings.js
 
-const util = require('util'),
-      ruleId = require("../myUtil.js").getRuleId(),
-      debug = require("debug")("apigeelint:" + ruleId);
+const util = require("util"),
+  ruleId = require("../myUtil.js").getRuleId(),
+  debug = require("debug")("apigeelint:" + ruleId);
 
 const plugin = {
-      ruleId : require("../myUtil.js").getRuleId(),
-      name: "unterminatedStrings",
-      message: "Conditions should not include strings that are unterminated.",
-      fatal: false,
-      severity: 2, //error
-      nodeType: "Condition",
-      enabled: true
-    };
-
-const isOdd = num => num % 2;
+  ruleId,
+  name: "unterminatedStrings",
+  message: "Conditions should not include strings that are unterminated.",
+  fatal: false,
+  severity: 2, //error
+  nodeType: "Condition",
+  enabled: true,
+};
 
-const onCondition = function(condition, cb) {
-        let truthTable = condition.getTruthTable(),
-            values = truthTable.getVals(),
-            tokens = truthTable.getTokens(),
-            source = condition.getSource(),
-            ast = truthTable.getAST(),
-            flagged = false;
+const isOdd = (num) => num % 2;
 
-        try {
-        debug(`source(${source}) values(${values})`);
-        debug(`values(` + JSON.stringify(values) + `)`);
-        debug(`tokens(` + JSON.stringify(tokens) + `)`);
-        debug(`ast(` + JSON.stringify(ast, null, 2) + `)`);
+const onCondition = function (condition, cb) {
+  let truthTable = condition.getTruthTable(),
+    values = truthTable.getVals(),
+    tokens = truthTable.getTokens(),
+    source = condition.getSource(),
+    ast = truthTable.getAST(),
+    flagged = false;
 
-        // let invalid = tokens.filter( t => t.valid == false);
-        // invalid.forEach( t => {
-        //   condition.addMessage({
-        //     source: condition.getExpression(),
-        //     line: condition.getElement().lineNumber,
-        //     column: condition.getElement().columnNumber,
-        //     plugin,
-        //     message: `invalid token. (${t.value})`
-        //   });
-        //   flagged = true;
-        // });
+  try {
+    debug(`source(${source}) values(${values})`);
+    debug(`values(` + JSON.stringify(values) + `)`);
+    debug(`tokens(` + JSON.stringify(tokens) + `)`);
+    debug(`ast(` + JSON.stringify(ast, null, 2) + `)`);
 
-        let constants = tokens.filter( t => t.type == 'constant');
-        constants.forEach( t => {
-          t.value = String(t.value);
-          if (t.value &&
-              ((t.value.endsWith('"') && !t.value.startsWith('"')) ||
-               ( !t.value.endsWith('"') && t.value.startsWith('"')) )
-             ) {
-          condition.addMessage({
-            source: condition.getExpression(),
-            line: condition.getElement().lineNumber,
-            column: condition.getElement().columnNumber,
-            plugin,
-            message: `Possible unterminated string: (${t.value})`
-          });
-          flagged = true;
-          }
-        });
+    // let invalid = tokens.filter( t => t.valid == false);
+    // invalid.forEach( t => {
+    //   condition.addMessage({
+    //     source: condition.getExpression(),
+    //     line: condition.getElement().lineNumber,
+    //     column: condition.getElement().columnNumber,
+    //     plugin,
+    //     message: `invalid token. (${t.value})`
+    //   });
+    //   flagged = true;
+    // });
 
-        let boundaries = tokens.filter( t => t.type == 'boundary');
-        debug(`boundaries(` + JSON.stringify(boundaries) + `)`);
-        debug(`boundaries.length(${boundaries.length})`);
-        if (isOdd(boundaries.length)) {
+    let constants = tokens.filter((t) => t.type == "constant");
+    constants.forEach((t) => {
+      debug(
+        `constant type(${typeof t.value}) value(${JSON.stringify(t.value)})`,
+      );
+      if (typeof t.value == "string") {
+        if (
+          (t.value.endsWith('"') && !t.value.startsWith('"')) ||
+          (!t.value.endsWith('"') && t.value.startsWith('"'))
+        ) {
           condition.addMessage({
             source: condition.getExpression(),
             line: condition.getElement().lineNumber,
             column: condition.getElement().columnNumber,
             plugin,
-            message: "unmatched parenthesis - possibly due to an unterminated string"
+            message: `Possible unterminated string: (${t.value})`,
           });
           flagged = true;
-
-        }
-        }
-        catch (e) {
-          console.error('exception: ' + e);
         }
+      }
+    });
+
+    let boundaries = tokens.filter((t) => t.type == "boundary");
+    debug(`boundaries(` + JSON.stringify(boundaries) + `)`);
+    debug(`boundaries.length(${boundaries.length})`);
+    if (isOdd(boundaries.length)) {
+      condition.addMessage({
+        source: condition.getExpression(),
+        line: condition.getElement().lineNumber,
+        column: condition.getElement().columnNumber,
+        plugin,
+        message:
+          "unmatched parenthesis - possibly due to an unterminated string",
+      });
+      flagged = true;
+    }
+  } catch (e) {
+    console.error("exception: " + e);
+  }
   if (typeof cb == "function") {
     cb(null, flagged);
   }
 };
 
 module.exports = {
   plugin,
-  onCondition
+  onCondition,
 };
diff --git a/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageAPIKeyFault.xml b/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageAPIKeyFault.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<AssignMessage async="false" continueOnError="false" enabled="true" name="AssignMessageAPIKeyFault">
+    <DisplayName>AssignMessageAPIKeyFault</DisplayName>
+    <AssignVariable>
+        <Name>user.message</Name>
+        <Value>The application is not authorized to make this request</Value>
+    </AssignVariable>
+    <IgnoreUnresolvedVariables>true</IgnoreUnresolvedVariables>
+</AssignMessage>
diff --git a/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageAddCORS.xml b/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageAddCORS.xml
@@ -0,0 +1,29 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<AssignMessage async="false" continueOnError="false" enabled="true" name="AssignMessageAddCORS">
+    <DisplayName>AssignMessageAddCORS</DisplayName>
+    <FaultRules>
+        <FaultRule name="serviceCalloutError">
+            <Step>
+                <Name>AssignServiceCalloutErrorResponse</Name>
+            </Step>
+            <Step>
+                <Name>InitializeResponseLog</Name>
+            </Step>
+            <Step>
+                <Name>FlowCalloutLogging</Name>
+            </Step>
+        </FaultRule>
+    </FaultRules>
+    <Properties />
+    <Set>
+        <Headers>
+            <Header name="Access-Control-Allow-Origin">*</Header>
+            <Header name="Access-Control-Allow-Headers">origin, x-requested-with, authorization,
+                accept, content-type, client_id, X-NW-Message-ID</Header>
+            <Header name="Access-Control-Max-Age">3628800</Header>
+            <Header name="Access-Control-Allow-Methods">GET, PUT, POST, OPTIONS</Header>
+        </Headers>
+    </Set>
+    <IgnoreUnresolvedVariables>true</IgnoreUnresolvedVariables>
+    <AssignTo createNew="false" transport="http" type="response" />
+</AssignMessage>
diff --git a/...s/resources/CC005/issue483/apiproxy/policies/AssignMessageDestinationUnreachableFault.xml b/...s/resources/CC005/issue483/apiproxy/policies/AssignMessageDestinationUnreachableFault.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<AssignMessage async="false" continueOnError="false" enabled="true" name="AssignMessageDestinationUnreachableFault">
+    <DisplayName>AssignMessageDestinationUnreachableFault</DisplayName>
+    <AssignVariable>
+        <Name>user.message</Name>
+        <Value>Service unavailable - please try again later</Value>
+    </AssignVariable>
+    <IgnoreUnresolvedVariables>true</IgnoreUnresolvedVariables>
+</AssignMessage>
diff --git a/...ixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageGenerateFaultResponse.xml b/...ixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageGenerateFaultResponse.xml
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<AssignMessage async="false" continueOnError="false" enabled="true" name="AssignMessageGenerateFaultResponse">
+    <DisplayName>AssignMessageGenerateFaultResponse</DisplayName>
+    <Set>
+        <Headers>
+            <Header name="Content-Type">application/json</Header>
+        </Headers>
+        <Payload contentType="application/json">
+            {   "userMessage": "{user.message}",
+                "developerMessage": {error.content},
+                "messageID": "{request.header.X-NW-Message-ID}",
+                "code": {error.status.code}
+            }
+        </Payload>
+    </Set>
+    <IgnoreUnresolvedVariables>true</IgnoreUnresolvedVariables>
+    <AssignTo createNew="false" transport="http" type="response"/>
+</AssignMessage>
diff --git a/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageGenericFault.xml b/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageGenericFault.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<AssignMessage async="false" continueOnError="false" enabled="true" name="AssignMessageGenericFault">
+    <DisplayName>AssignMessageGenericFault</DisplayName>
+    <AssignVariable>
+        <Name>user.message</Name>
+        <Value>Server error</Value>
+    </AssignVariable>
+    <IgnoreUnresolvedVariables>true</IgnoreUnresolvedVariables>
+</AssignMessage>
diff --git a/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageGenericOauthError.xml b/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageGenericOauthError.xml
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<AssignMessage async="false" continueOnError="false" enabled="true" name="AssignMessageGenericOauthError">
+    <DisplayName>AssignMessageGenericOauthError</DisplayName>
+    <AssignVariable>
+        <Name>user.message</Name>
+        <Value>The user is not authorized to make this request</Value>
+    </AssignVariable>
+    <IgnoreUnresolvedVariables>true</IgnoreUnresolvedVariables>
+    <AssignTo createNew="false" transport="http" type="request"/>
+</AssignMessage>
diff --git a/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageJSTimeoutFault.xml b/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageJSTimeoutFault.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<AssignMessage async="false" continueOnError="false" enabled="true" name="AssignMessageJSTimeoutFault">
+    <DisplayName>AssignMessageJSTimeoutFault</DisplayName>
+    <AssignVariable>
+        <Name>user.message</Name>
+        <Value>Javscript execution has exceeded the time limit set.</Value>
+    </AssignVariable>
+    <IgnoreUnresolvedVariables>true</IgnoreUnresolvedVariables>
+</AssignMessage>
diff --git a/...ixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageRemoveSecurityHeaders.xml b/...ixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageRemoveSecurityHeaders.xml
@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<AssignMessage async="false" continueOnError="false" enabled="true" name="AssignMessageRemoveSecurityHeaders">
+    <DisplayName>AssignMessageRemoveSecurityHeaders</DisplayName>
+    <Properties/>
+    <Remove>
+        <Headers>
+            <Header name="client_id"/>
+            <Header name="Authorization"/>
+        </Headers>
+    </Remove>
+    <IgnoreUnresolvedVariables>true</IgnoreUnresolvedVariables>
+    <AssignTo createNew="false" transport="http" type="request"/>
+</AssignMessage>
diff --git a/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageResourceNotFound.xml b/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageResourceNotFound.xml
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<AssignMessage async="false" continueOnError="false" enabled="true" name="AssignMessageResourceNotFound">
+    <DisplayName>AssignMessageResourceNotFound</DisplayName>
+    <Properties/>
+    <Set>
+        <!-- Note: the payload is the error.content, so it can be set in AssignVariable
+             or in <Set> <Payload> depending on how complex the value needs to be.  In
+             this example a combination of string literal with variable substitution was
+             desired, so the <Payload> approach was used. -->
+        <Payload>Resource name: {proxy.pathsuffix}, method: {request.verb}</Payload>
+    </Set>
+    <AssignVariable>
+        <Name>user.message</Name>
+        <Value>The resource requested was not found</Value>
+    </AssignVariable>
+    <IgnoreUnresolvedVariables>true</IgnoreUnresolvedVariables>
+</AssignMessage>
diff --git a/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageSQLInjectionFault.xml b/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageSQLInjectionFault.xml
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<AssignMessage async="false" continueOnError="false" enabled="true" name="AssignMessageSQLInjectionFault">
+    <DisplayName>AssignMessageSQLInjectionFault</DisplayName>
+    <AssignVariable>
+        <Name>user.message</Name>
+        <Value>Bad Request</Value>
+    </AssignVariable>
+    <IgnoreUnresolvedVariables>true</IgnoreUnresolvedVariables>
+    <AssignTo createNew="false" transport="http" type="request"/>
+</AssignMessage>
diff --git a/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageSetTargetHost.xml b/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageSetTargetHost.xml
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<AssignMessage async="false" continueOnError="false" enabled="true" name="AssignMessageSetTargetHost">
+    <DisplayName>AssignMessageSetTargetHost</DisplayName>
+    <Set>
+        <Headers>
+            <Header name="X-NW-Target-URL">{target-url}{proxy.pathsuffix}?{request.querystring}</Header>
+            <Header name="X-NW-Internal-FWD">false</Header>
+        </Headers>
+    </Set>
+    <AssignVariable>
+        <Name>target.copy.pathsuffix</Name>
+        <Value>false</Value>
+    </AssignVariable>
+    <AssignVariable>
+        <Name>target.copy.queryparams</Name>
+        <Value>false</Value>
+    </AssignVariable>
+    <IgnoreUnresolvedVariables>true</IgnoreUnresolvedVariables>
+    <AssignTo createNew="false" transport="http" type="request"/>
+</AssignMessage>
diff --git a/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageSpikeArrestFault.xml b/test/fixtures/resources/CC005/issue483/apiproxy/policies/AssignMessageSpikeArrestFault.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<AssignMessage async="false" continueOnError="false" enabled="true" name="AssignMessageSpikeArrestFault">
+    <DisplayName>AssignMessageSpikeArrestFault</DisplayName>
+    <Properties/>
+    <Set>
+        <StatusCode>429</StatusCode>
+        <ReasonPhrase>Spike Arrest Rate Limiting</ReasonPhrase>
+    </Set>
+    <AssignVariable>
+        <Name>user.message</Name>
+        <Value>Too many requests in a given amount of time</Value>
+    </AssignVariable>
+    <IgnoreUnresolvedVariables>true</IgnoreUnresolvedVariables>
+    <AssignTo createNew="false" transport="http" type="request"/>
+</AssignMessage>
diff --git a/test/fixtures/resources/CC005/issue483/apiproxy/policies/FlowCalloutLogging.xml b/test/fixtures/resources/CC005/issue483/apiproxy/policies/FlowCalloutLogging.xml
@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<FlowCallout async="false" continueOnError="false" enabled="true" name="FlowCalloutLogging">
+    <DisplayName>FlowCalloutLogging</DisplayName>
+    <FaultRules/>
+    <Properties/>
+    <SharedFlowBundle>EnterpriseLogging_1</SharedFlowBundle>
+</FlowCallout>
diff --git a/test/fixtures/resources/CC005/issue483/apiproxy/policies/JSONThreatProtection.xml b/test/fixtures/resources/CC005/issue483/apiproxy/policies/JSONThreatProtection.xml
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<JSONThreatProtection async="false" continueOnError="false" enabled="true" name="JSONThreatProtection">
+    <DisplayName>JSONThreatProtection</DisplayName>
+    <Properties/>
+    <ArrayElementCount>20</ArrayElementCount>
+    <ContainerDepth>10</ContainerDepth>
+    <ObjectEntryCount>15</ObjectEntryCount>
+    <ObjectEntryNameLength>50</ObjectEntryNameLength>
+    <Source>request</Source>
+    <StringValueLength>500</StringValueLength>
+</JSONThreatProtection>
diff --git a/test/fixtures/resources/CC005/issue483/apiproxy/policies/JavaScriptCatchJSErrors.xml b/test/fixtures/resources/CC005/issue483/apiproxy/policies/JavaScriptCatchJSErrors.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<Javascript timeLimit="120000" async="false" continueOnError="false" enabled="true" name="JavaScriptCatchJSErrors">
+    <DisplayName>JavaScriptCatchJSErrors</DisplayName>
+    <FaultRules/>
+    <Properties/>
+    <IncludeURL>jsc://common.js</IncludeURL>
+    <ResourceURL>jsc://catchJSErrors.js</ResourceURL>
+</Javascript>
diff --git a/test/fixtures/resources/CC005/issue483/apiproxy/policies/JavaScriptValidateRequest.xml b/test/fixtures/resources/CC005/issue483/apiproxy/policies/JavaScriptValidateRequest.xml
@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<Javascript async="false" continueOnError="false" enabled="true" timeLimit="120000" name="JavaScriptValidateRequest">
+    <DisplayName>JavaScriptValidateRequest</DisplayName>
+    <Properties/>
+    <IncludeURL>jsc://common.js</IncludeURL>
+    <ResourceURL>jsc://validateRequest.js</ResourceURL>
+</Javascript>