Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

escapeJavaScript doesn't escape quotes #8

Open
jlongman opened this issue Nov 8, 2021 · 1 comment
Open

escapeJavaScript doesn't escape quotes #8

jlongman opened this issue Nov 8, 2021 · 1 comment

Comments

@jlongman
Copy link

jlongman commented Nov 8, 2021

It looks to me like the table used to do the conversion has the wrong value for the " symbol. see https://github.com/ToQoz/api-gateway-mapping-template/blob/master/index.js#L164

var escapeJavaScriptTable = {
  '"': '\"',    // 2.a
  '\\': '\\\\',

but should be

var escapeJavaScriptTable = {
  '"': '\\"',    // 2.a
  '\\': '\\\\',

note the extra backslash. I tested this in a debugger and overrode the value in the table to the doubly-escaped value and my tests passed. Additionally I sent it to apigw->dynamodb and the value was correct.


Given, in the template:
"categories": {"S": "$util.escapeJavaScript($input.json('$.categories'))"}
I expect my output to look like:
"categories":{\"pwa\":{\"id\":\"pwa\",\"title\":\"Progressive Web App\"[...]
but it looks like
"categories":{"pwa":{"id":"pwa","title":"Progressive Web App"[...]

My test looks like:

test('complete APRT VLT', () => {
  const context = {
    requestId: 'externally-specified-value',
  };
  const categories = {
    pwa: {
      id: 'pwa',
      title: 'Progressive Web App',
      score: '0.33',
    },
//.. cut
  };
  const body = {
    auditId: 'body-specified-value',
    categories,
//.. cut
  };
  const jsonBody = JSON.stringify(body);
  const result = mappingTemplate({ template: vtl, payload: jsonBody, context });
  expect(result).toBeDefined();
  expect(result).not.toEqual('');
  const json = JSON.parse(result);
  expect(json).toBeDefined();
  expect(json.categories).toBe(categories); // TODO richer test suite

Using the same template with APIGW to dynamodDB, when I push the object then do a GetItem on it I get:

{
    "Item": {
        "auditId": {
            "S": "09fe9c66-a508-447c-8878-a7de1503e88c"
        },
        "categories": {
            "S": "{\"pwa\":{\"id\":\"pwa\",\"title\":\"Progressive Web App\",\"score\":0.33}, //etc
        },

Ie the object is correctly escaped.

@jlongman
Copy link
Author

jlongman commented Nov 8, 2021

I am happy to create a PR, but how active is this project?

I note that 0.0.8 is listed in npm but only 0.07 is shown as released on github?

jlongman added a commit to jlongman/api-gateway-mapping-template that referenced this issue Nov 8, 2021
ref: ToQoz#8

double quote the string used in `util.escapeJsonString`.

Note, I notice the existing test - https://github.com/jlongman/api-gateway-mapping-template/blob/master/test/util.js#L5 - this would presumably break this test? I may add the test in the original issue to the PR.

Thinking about this, maybe this suggests a difference in runtime environments?  Will add notes to the issue.
jlongman added a commit to jlongman/api-gateway-mapping-template that referenced this issue Nov 8, 2021
ref: ToQoz#8

double quote the string used in `util.escapeJsonString`.

Note, I notice the existing test - https://github.com/jlongman/api-gateway-mapping-template/blob/master/test/util.js#L5 - this would presumably break this test? I may add the test in the original issue to the PR.

Thinking about this, maybe this suggests a difference in runtime environments?  Will add notes to the issue.
jlongman added a commit to jlongman/api-gateway-mapping-template that referenced this issue Nov 9, 2021
ref: ToQoz#8

double quote the string used in `util.escapeJsonString`.

Note, I notice the existing test - https://github.com/jlongman/api-gateway-mapping-template/blob/master/test/util.js#L5 - this would presumably break this test? I may add the test in the original issue to the PR.

Thinking about this, maybe this suggests a difference in runtime environments?  Will add notes to the issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant