fix(apigateway): CORS response body has unexpected space (#27219)

Because of the way Velocity templates are parsed, the space just before `#end` in the original template was emitted literally instead of swallowed. This caused the `OPTIONS` response to contain a single space (`" "`), which makes some validation tools reject the response as invalid. Rewrite the Velocity template to a syntax we know doesn't cause that space to be emitted. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
aws · Sep 21, 2023 · abf21c9 · abf21c9
1 parent 7d02e5e
commit abf21c9
Show file tree

Hide file tree

Showing 10 changed files with 67 additions and 26 deletions.
diff --git a/...nteg/test/aws-apigateway/test/integ.cors-allow-multiple-origins.js.snapshot/manifest.json b/...nteg/test/aws-apigateway/test/integ.cors-allow-multiple-origins.js.snapshot/manifest.json
@@ -14,10 +14,11 @@
       "environment": "aws://unknown-account/unknown-region",
       "properties": {
         "templateFile": "stack-cors-allow-multiple-origins.template.json",
+        "terminationProtection": false,
         "validateOnSynth": false,
         "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
         "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
-        "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/0cc1681b59d7456fa47ee61cb91884542f825e1d36afb4b442a57bf959e4d742.json",
+        "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/10b38de28f6f291a0a2fe437096ce3ea526ffeb14c593b034d0df5e8452cde82.json",
         "requiresBootstrapStackVersion": 6,
         "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
         "additionalDependencies": [
@@ -42,7 +43,7 @@
         "/stack-cors-allow-multiple-origins/cors-api-test/Deployment/Resource": [
           {
             "type": "aws:cdk:logicalId",
-            "data": "corsapitestDeployment2BF1633Ad72428c37c88b8c23ef39eebb5b7e9fd"
+            "data": "corsapitestDeployment2BF1633A506e17bacbb92c0f9e65d8c89b158b9c"
           }
         ],
         "/stack-cors-allow-multiple-origins/cors-api-test/DeploymentStage.prod/Resource": [
@@ -116,6 +117,15 @@
             "type": "aws:cdk:logicalId",
             "data": "CheckBootstrapVersion"
           }
+        ],
+        "corsapitestDeployment2BF1633Ad72428c37c88b8c23ef39eebb5b7e9fd": [
+          {
+            "type": "aws:cdk:logicalId",
+            "data": "corsapitestDeployment2BF1633Ad72428c37c88b8c23ef39eebb5b7e9fd",
+            "trace": [
+              "!!DESTRUCTIVE_CHANGES: WILL_DESTROY"
+            ]
+          }
         ]
       },
       "displayName": "stack-cors-allow-multiple-origins"
@@ -133,6 +143,7 @@
       "environment": "aws://unknown-account/unknown-region",
       "properties": {
         "templateFile": "integcorsallowmultipleoriginsDefaultTestDeployAssertEBF0A1B1.template.json",
+        "terminationProtection": false,
         "validateOnSynth": false,
         "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
         "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",

diff --git a/...teg.cors-allow-multiple-origins.js.snapshot/stack-cors-allow-multiple-origins.assets.json b/...teg.cors-allow-multiple-origins.js.snapshot/stack-cors-allow-multiple-origins.assets.json
@@ -14,15 +14,15 @@
         }
       }
     },
-    "0cc1681b59d7456fa47ee61cb91884542f825e1d36afb4b442a57bf959e4d742": {
+    "10b38de28f6f291a0a2fe437096ce3ea526ffeb14c593b034d0df5e8452cde82": {
       "source": {
         "path": "stack-cors-allow-multiple-origins.template.json",
         "packaging": "file"
       },
       "destinations": {
         "current_account-current_region": {
           "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
-          "objectKey": "0cc1681b59d7456fa47ee61cb91884542f825e1d36afb4b442a57bf959e4d742.json",
+          "objectKey": "10b38de28f6f291a0a2fe437096ce3ea526ffeb14c593b034d0df5e8452cde82.json",
           "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
         }
       }

diff --git a/...g.cors-allow-multiple-origins.js.snapshot/stack-cors-allow-multiple-origins.template.json b/...g.cors-allow-multiple-origins.js.snapshot/stack-cors-allow-multiple-origins.template.json
@@ -6,7 +6,7 @@
     "Name": "cors-api-test"
    }
   },
-  "corsapitestDeployment2BF1633Ad72428c37c88b8c23ef39eebb5b7e9fd": {
+  "corsapitestDeployment2BF1633A506e17bacbb92c0f9e65d8c89b158b9c": {
    "Type": "AWS::ApiGateway::Deployment",
    "Properties": {
     "Description": "Automatically created by the RestApi construct",
@@ -25,7 +25,7 @@
    "Type": "AWS::ApiGateway::Stage",
    "Properties": {
     "DeploymentId": {
-     "Ref": "corsapitestDeployment2BF1633Ad72428c37c88b8c23ef39eebb5b7e9fd"
+     "Ref": "corsapitestDeployment2BF1633A506e17bacbb92c0f9e65d8c89b158b9c"
     },
     "RestApiId": {
      "Ref": "corsapitest8682546E"
@@ -49,7 +49,7 @@
         "method.response.header.Access-Control-Allow-Methods": "'OPTIONS,GET,PUT,POST,DELETE,PATCH,HEAD'"
        },
        "ResponseTemplates": {
-        "application/json": "#set($origin = $input.params().header.get(\"Origin\"))\n#if($origin == \"\") #set($origin = $input.params().header.get(\"origin\")) #end\n#if($origin == \"https://twitch.tv\" || $origin == \"https://aws.amazon.com\")\n  #set($context.responseOverride.header.Access-Control-Allow-Origin = $origin)\n#end"
+        "application/json": "#set($origin = $input.params().header.get(\"Origin\"))\n#if($origin == \"\")\n  #set($origin = $input.params().header.get(\"origin\"))\n#end\n#if($origin == \"https://twitch.tv\" || $origin == \"https://aws.amazon.com\")\n  #set($context.responseOverride.header.Access-Control-Allow-Origin = $origin)\n#end"
        },
        "StatusCode": "204"
       }
@@ -112,7 +112,7 @@
         "method.response.header.Access-Control-Allow-Methods": "'OPTIONS,GET,PUT,POST,DELETE,PATCH,HEAD'"
        },
        "ResponseTemplates": {
-        "application/json": "#set($origin = $input.params().header.get(\"Origin\"))\n#if($origin == \"\") #set($origin = $input.params().header.get(\"origin\")) #end\n#if($origin == \"https://twitch.tv\" || $origin == \"https://aws.amazon.com\")\n  #set($context.responseOverride.header.Access-Control-Allow-Origin = $origin)\n#end"
+        "application/json": "#set($origin = $input.params().header.get(\"Origin\"))\n#if($origin == \"\")\n  #set($origin = $input.params().header.get(\"origin\"))\n#end\n#if($origin == \"https://twitch.tv\" || $origin == \"https://aws.amazon.com\")\n  #set($context.responseOverride.header.Access-Control-Allow-Origin = $origin)\n#end"
        },
        "StatusCode": "204"
       }

diff --git a/...rk-integ/test/aws-apigateway/test/integ.cors-allow-multiple-origins.js.snapshot/tree.json b/...rk-integ/test/aws-apigateway/test/integ.cors-allow-multiple-origins.js.snapshot/tree.json
@@ -64,7 +64,7 @@
                       "aws:cdk:cloudformation:type": "AWS::ApiGateway::Stage",
                       "aws:cdk:cloudformation:props": {
                         "deploymentId": {
-                          "Ref": "corsapitestDeployment2BF1633Ad72428c37c88b8c23ef39eebb5b7e9fd"
+                          "Ref": "corsapitestDeployment2BF1633A506e17bacbb92c0f9e65d8c89b158b9c"
                         },
                         "restApiId": {
                           "Ref": "corsapitest8682546E"
@@ -123,7 +123,7 @@
                                     "method.response.header.Access-Control-Allow-Methods": "'OPTIONS,GET,PUT,POST,DELETE,PATCH,HEAD'"
                                   },
                                   "responseTemplates": {
-                                    "application/json": "#set($origin = $input.params().header.get(\"Origin\"))\n#if($origin == \"\") #set($origin = $input.params().header.get(\"origin\")) #end\n#if($origin == \"https://twitch.tv\" || $origin == \"https://aws.amazon.com\")\n  #set($context.responseOverride.header.Access-Control-Allow-Origin = $origin)\n#end"
+                                    "application/json": "#set($origin = $input.params().header.get(\"Origin\"))\n#if($origin == \"\")\n  #set($origin = $input.params().header.get(\"origin\"))\n#end\n#if($origin == \"https://twitch.tv\" || $origin == \"https://aws.amazon.com\")\n  #set($context.responseOverride.header.Access-Control-Allow-Origin = $origin)\n#end"
                                   }
                                 }
                               ]
@@ -216,7 +216,7 @@
                                         "method.response.header.Access-Control-Allow-Methods": "'OPTIONS,GET,PUT,POST,DELETE,PATCH,HEAD'"
                                       },
                                       "responseTemplates": {
-                                        "application/json": "#set($origin = $input.params().header.get(\"Origin\"))\n#if($origin == \"\") #set($origin = $input.params().header.get(\"origin\")) #end\n#if($origin == \"https://twitch.tv\" || $origin == \"https://aws.amazon.com\")\n  #set($context.responseOverride.header.Access-Control-Allow-Origin = $origin)\n#end"
+                                        "application/json": "#set($origin = $input.params().header.get(\"Origin\"))\n#if($origin == \"\")\n  #set($origin = $input.params().header.get(\"origin\"))\n#end\n#if($origin == \"https://twitch.tv\" || $origin == \"https://aws.amazon.com\")\n  #set($context.responseOverride.header.Access-Control-Allow-Origin = $origin)\n#end"
                                       }
                                     }
                                   ]
@@ -576,7 +576,7 @@
                 "path": "integ-cors-allow-multiple-origins/DefaultTest/Default",
                 "constructInfo": {
                   "fqn": "constructs.Construct",
-                  "version": "10.2.69"
+                  "version": "10.2.70"
                 }
               },
               "DeployAssert": {
@@ -622,7 +622,7 @@
         "path": "Tree",
         "constructInfo": {
           "fqn": "constructs.Construct",
-          "version": "10.2.69"
+          "version": "10.2.70"
         }
       }
     },

diff --git a/...mework-integ/test/aws-apigateway/test/integ.cors.js.snapshot/cors-twitch-test.assets.json b/...mework-integ/test/aws-apigateway/test/integ.cors.js.snapshot/cors-twitch-test.assets.json
@@ -14,15 +14,15 @@
         }
       }
     },
-    "2f8b518aaca6753a8d9097bb7f4c058c8e83ebed2d2446f593695bb69f478fc3": {
+    "3b86b43c1a293038e6b507b773f348d699167e46c088fe6ed4b1ae400ce17192": {
       "source": {
         "path": "cors-twitch-test.template.json",
         "packaging": "file"
       },
       "destinations": {
         "current_account-current_region": {
           "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
-          "objectKey": "2f8b518aaca6753a8d9097bb7f4c058c8e83ebed2d2446f593695bb69f478fc3.json",
+          "objectKey": "3b86b43c1a293038e6b507b773f348d699167e46c088fe6ed4b1ae400ce17192.json",
           "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
         }
       }

diff --git a/...work-integ/test/aws-apigateway/test/integ.cors.js.snapshot/cors-twitch-test.template.json b/...work-integ/test/aws-apigateway/test/integ.cors.js.snapshot/cors-twitch-test.template.json
@@ -55,7 +55,7 @@
    "UpdateReplacePolicy": "Retain",
    "DeletionPolicy": "Retain"
   },
-  "corsapitestDeployment2BF1633Adc24ac182461aecc920a8724663ce2a1": {
+  "corsapitestDeployment2BF1633A72d0197fd6797ee251232efbdce56b8a": {
    "Type": "AWS::ApiGateway::Deployment",
    "Properties": {
     "Description": "Automatically created by the RestApi construct",
@@ -75,7 +75,7 @@
    "Type": "AWS::ApiGateway::Stage",
    "Properties": {
     "DeploymentId": {
-     "Ref": "corsapitestDeployment2BF1633Adc24ac182461aecc920a8724663ce2a1"
+     "Ref": "corsapitestDeployment2BF1633A72d0197fd6797ee251232efbdce56b8a"
     },
     "RestApiId": {
      "Ref": "corsapitest8682546E"
@@ -471,7 +471,7 @@
         "method.response.header.Access-Control-Allow-Methods": "'OPTIONS,GET,PUT,POST,DELETE,PATCH,HEAD'"
        },
        "ResponseTemplates": {
-        "application/json": "#set($origin = $input.params().header.get(\"Origin\"))\n#if($origin == \"\") #set($origin = $input.params().header.get(\"origin\")) #end\n#if($origin == \"https://www.test-cors.org\")\n  #set($context.responseOverride.header.Access-Control-Allow-Origin = $origin)\n#end"
+        "application/json": "#set($origin = $input.params().header.get(\"Origin\"))\n#if($origin == \"\")\n  #set($origin = $input.params().header.get(\"origin\"))\n#end\n#if($origin == \"https://www.test-cors.org\")\n  #set($context.responseOverride.header.Access-Control-Allow-Origin = $origin)\n#end"
        },
        "StatusCode": "204"
       }

diff --git a/...cdk-testing/framework-integ/test/aws-apigateway/test/integ.cors.js.snapshot/manifest.json b/...cdk-testing/framework-integ/test/aws-apigateway/test/integ.cors.js.snapshot/manifest.json
@@ -14,10 +14,11 @@
       "environment": "aws://unknown-account/unknown-region",
       "properties": {
         "templateFile": "cors-twitch-test.template.json",
+        "terminationProtection": false,
         "validateOnSynth": false,
         "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
         "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
-        "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/2f8b518aaca6753a8d9097bb7f4c058c8e83ebed2d2446f593695bb69f478fc3.json",
+        "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/3b86b43c1a293038e6b507b773f348d699167e46c088fe6ed4b1ae400ce17192.json",
         "requiresBootstrapStackVersion": 6,
         "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
         "additionalDependencies": [
@@ -54,7 +55,7 @@
         "/cors-twitch-test/cors-api-test/Deployment/Resource": [
           {
             "type": "aws:cdk:logicalId",
-            "data": "corsapitestDeployment2BF1633Adc24ac182461aecc920a8724663ce2a1"
+            "data": "corsapitestDeployment2BF1633A72d0197fd6797ee251232efbdce56b8a"
           }
         ],
         "/cors-twitch-test/cors-api-test/DeploymentStage.prod/Resource": [
@@ -158,6 +159,15 @@
             "type": "aws:cdk:logicalId",
             "data": "CheckBootstrapVersion"
           }
+        ],
+        "corsapitestDeployment2BF1633Adc24ac182461aecc920a8724663ce2a1": [
+          {
+            "type": "aws:cdk:logicalId",
+            "data": "corsapitestDeployment2BF1633Adc24ac182461aecc920a8724663ce2a1",
+            "trace": [
+              "!!DESTRUCTIVE_CHANGES: WILL_DESTROY"
+            ]
+          }
         ]
       },
       "displayName": "cors-twitch-test"
@@ -175,6 +185,7 @@
       "environment": "aws://unknown-account/unknown-region",
       "properties": {
         "templateFile": "corsDefaultTestDeployAssert5CF8F851.template.json",
+        "terminationProtection": false,
         "validateOnSynth": false,
         "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
         "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",

diff --git a/...aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.cors.js.snapshot/tree.json b/...aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.cors.js.snapshot/tree.json
@@ -140,7 +140,7 @@
                       "aws:cdk:cloudformation:type": "AWS::ApiGateway::Stage",
                       "aws:cdk:cloudformation:props": {
                         "deploymentId": {
-                          "Ref": "corsapitestDeployment2BF1633Adc24ac182461aecc920a8724663ce2a1"
+                          "Ref": "corsapitestDeployment2BF1633A72d0197fd6797ee251232efbdce56b8a"
                         },
                         "restApiId": {
                           "Ref": "corsapitest8682546E"
@@ -682,7 +682,7 @@
                                         "method.response.header.Access-Control-Allow-Methods": "'OPTIONS,GET,PUT,POST,DELETE,PATCH,HEAD'"
                                       },
                                       "responseTemplates": {
-                                        "application/json": "#set($origin = $input.params().header.get(\"Origin\"))\n#if($origin == \"\") #set($origin = $input.params().header.get(\"origin\")) #end\n#if($origin == \"https://www.test-cors.org\")\n  #set($context.responseOverride.header.Access-Control-Allow-Origin = $origin)\n#end"
+                                        "application/json": "#set($origin = $input.params().header.get(\"Origin\"))\n#if($origin == \"\")\n  #set($origin = $input.params().header.get(\"origin\"))\n#end\n#if($origin == \"https://www.test-cors.org\")\n  #set($context.responseOverride.header.Access-Control-Allow-Origin = $origin)\n#end"
                                       }
                                     }
                                   ]

diff --git a/packages/aws-cdk-lib/aws-apigateway/lib/resource.ts b/packages/aws-cdk-lib/aws-apigateway/lib/resource.ts
@@ -317,8 +317,11 @@ export abstract class ResourceBase extends ResourceConstruct implements IResourc
 
       const template = new Array<string>();
 
-      template.push('#set($origin = $input.params().header.get("Origin"))');
-      template.push('#if($origin == "") #set($origin = $input.params().header.get("origin")) #end');
+      template.push(
+        '#set($origin = $input.params().header.get("Origin"))',
+        '#if($origin == "")',
+        '  #set($origin = $input.params().header.get("origin"))',
+        '#end');
 
       const condition = origins.map(o => `$origin == "${o}"`).join(' || ');
 

diff --git a/packages/aws-cdk-lib/aws-apigateway/test/cors.test.ts b/packages/aws-cdk-lib/aws-apigateway/test/cors.test.ts
@@ -290,7 +290,15 @@ describe('cors', () => {
               'method.response.header.Access-Control-Allow-Methods': "'OPTIONS,GET,PUT,POST,DELETE,PATCH,HEAD'",
             },
             ResponseTemplates: {
-              'application/json': '#set($origin = $input.params().header.get("Origin"))\n#if($origin == "") #set($origin = $input.params().header.get("origin")) #end\n#if($origin == "https://amazon.com" || $origin == "https://aws.amazon.com")\n  #set($context.responseOverride.header.Access-Control-Allow-Origin = $origin)\n#end',
+              'application/json': [
+                '#set($origin = $input.params().header.get("Origin"))',
+                '#if($origin == "")',
+                '  #set($origin = $input.params().header.get("origin"))',
+                '#end',
+                '#if($origin == "https://amazon.com" || $origin == "https://aws.amazon.com")',
+                '  #set($context.responseOverride.header.Access-Control-Allow-Origin = $origin)',
+                '#end',
+              ].join('\n'),
             },
             StatusCode: '204',
           },
@@ -688,7 +696,15 @@ describe('cors', () => {
               'method.response.header.Access-Control-Allow-Methods': "'OPTIONS,GET,PUT,POST,DELETE,PATCH,HEAD'",
             },
             ResponseTemplates: {
-              'application/json': '#set($origin = $input.params().header.get("Origin"))\n#if($origin == "") #set($origin = $input.params().header.get("origin")) #end\n#if($origin == "https://twitch.tv")\n  #set($context.responseOverride.header.Access-Control-Allow-Origin = $origin)\n#end',
+              'application/json': [
+                '#set($origin = $input.params().header.get("Origin"))',
+                '#if($origin == "")',
+                '  #set($origin = $input.params().header.get("origin"))',
+                '#end',
+                '#if($origin == "https://twitch.tv")',
+                '  #set($context.responseOverride.header.Access-Control-Allow-Origin = $origin)',
+                '#end',
+              ].join('\n'),
             },
             StatusCode: '204',
           },