Skip to content

fix(parse): remove backspace control characters from input to be parsed #53

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 27, 2019
Merged

fix(parse): remove backspace control characters from input to be parsed #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion lib/parse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@
module.exports = function (input, callback) {
var result;
try {
result = JSON.parse(input);
// The input is a JSON string, which may contain control characters
// that should be removed. See LLK/scratch-vm#1077
// So far we've only encountered the backspace control character,
// so remove that specific one before continuing.
// SB2 JSONs and SB3 JSONs have different versions of the
// character serialized (e.g. \u0008 and \b), strip out both versions
result = JSON.parse(input.replace(/\\b|\\u0008/g, ''));
Copy link

@towerofnix towerofnix Feb 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this make backspace characters intentionally used as the input to a "key () pressed?" block break, because the input would be empty (and hence wouldn't detect backspace, as it was intended)?

Copy link
Contributor

@thisandagain thisandagain Feb 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this would remove backspace control characters that are intentionally added (via "hacked" arguments).

Copy link

@towerofnix towerofnix Feb 26, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, we would be "fixing" those projects in that they would now load, but they would in fact not operate the way they're meant to, correct? I understand that this PR also fixes issues with JP keyboard input, but does that outweigh the half-broken projects this PR would cause?

Copy link

@joker314 joker314 Feb 26, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this PR make a project owner who has an intentional backspace character, when opening their project in the editor view, override their project JSON to remove the backspace character permanently?

I think a fix to this issue is very important, and like the solution presented in this pull request, and I understand that such intentionally 'hacked' projects are a minority, but ideally we'd be able to preserve the functionality of those 'hacked' projects (or at least be able to restore that functionality at a later date).

} catch (e) {
return callback(e.toString());
}
Expand Down
96 changes: 96 additions & 0 deletions test/fixtures/data/217845144_sb2_control_char_in_strings_credit_to_Mr-Dave_test.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
{
"objName": "Stage",
"variables": [{
"name": "\u0008b-damage",
"value": 47.600000000000406,
"isPersistent": false
}],
"sounds": [{
"soundName": "pop",
"soundID": 1,
"md5": "83a9787d4cb6f3b7632b4ddfebf74367.wav",
"sampleCount": 258,
"rate": 11025,
"format": ""
}],
"costumes": [{
"costumeName": "backdrop1",
"baseLayerID": 3,
"baseLayerMD5": "739b5e2a2435f6e1ec2993791b423146.png",
"bitmapResolution": 1,
"rotationCenterX": 240,
"rotationCenterY": 180
}],
"currentCostumeIndex": 0,
"penLayerMD5": "5c81a336fab8be57adc039a8a2b33ca9.png",
"penLayerID": 0,
"tempoBPM": 60,
"videoAlpha": 0.5,
"children": [{
"objName": "Sprite1",
"scripts": [[178.4,
46.15,
[["whenGreenFlag"],
["setVar:to:", "\u0008b-damage", "2"],
["doForever", [["turnRight:", ["readVariable", "\u0008b-damage"]], ["changeVar:by:", "\u0008b-damage", 0.1]]]]]],
"sounds": [{
"soundName": "meow",
"soundID": 0,
"md5": "83c36d806dc92327b9e7049a565c6bff.wav",
"sampleCount": 18688,
"rate": 22050,
"format": ""
}],
"costumes": [{
"costumeName": "costume1",
"baseLayerID": 1,
"baseLayerMD5": "09dc888b0b7df19f70d81588ae73420e.svg",
"bitmapResolution": 1,
"rotationCenterX": 47,
"rotationCenterY": 55
},
{
"costumeName": "costume2",
"baseLayerID": 2,
"baseLayerMD5": "3696356a03a8d938318876a593572843.svg",
"bitmapResolution": 1,
"rotationCenterX": 47,
"rotationCenterY": 55
}],
"currentCostumeIndex": 0,
"scratchX": -12.550000000000011,
"scratchY": -22,
"scale": 1,
"direction": 105.79999999757408,
"rotationStyle": "normal",
"isDraggable": false,
"indexInLibrary": 1,
"visible": true,
"spriteInfo": {
}
},
{
"target": "Stage",
"cmd": "getVar:",
"param": "\u0008b-damage",
"color": 15629590,
"label": "\u0008b-damage",
"mode": 1,
"sliderMin": 0,
"sliderMax": 100,
"isDiscrete": true,
"x": 5,
"y": 5,
"visible": true
}],
"info": {
"userAgent": "Mozilla\/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit\/537.36 (KHTML, like Gecko) Chrome\/68.0.3440.106 Safari\/537.36",
"swfVersion": "v461",
"spriteCount": 1,
"videoOn": false,
"hasCloudData": false,
"scriptCount": 1,
"flashVersion": "MAC 31,0,0,108",
"projectID": "217845144"
}
}
1 change: 1 addition & 0 deletions test/fixtures/data/217845144_sb3_control_char_in_strings_credit_to_Mr-Dave_test.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"targets":[{"isStage":true,"name":"Stage","variables":{"|fg9Sax{3^M|yQ/@+Pb^-\bb-damage-":["\bb-damage",47.600000000000406]},"lists":{},"broadcasts":{},"blocks":{},"comments":{},"currentCostume":0,"costumes":[{"assetId":"797b03bdb8cf6ccfc30c0692d533d998","name":"backdrop1","bitmapResolution":2,"md5ext":"797b03bdb8cf6ccfc30c0692d533d998.png","dataFormat":"png","rotationCenterX":480,"rotationCenterY":360}],"sounds":[{"assetId":"83a9787d4cb6f3b7632b4ddfebf74367","name":"pop","dataFormat":"wav","format":"","rate":44100,"sampleCount":1032,"md5ext":"83a9787d4cb6f3b7632b4ddfebf74367.wav"}],"volume":100,"layerOrder":0,"tempo":60,"videoTransparency":50,"videoState":"off","textToSpeechLanguage":null},{"isStage":false,"name":"Sprite1","variables":{},"lists":{},"broadcasts":{},"blocks":{"Fo3;}RZh889lf|v)*Wc8":{"opcode":"event_whenflagclicked","next":"s.no,^PDr@)KV@#Z([zR","parent":null,"inputs":{},"fields":{},"shadow":false,"topLevel":true,"x":268,"y":102},"s.no,^PDr@)KV@#Z([zR":{"opcode":"data_setvariableto","next":"s7Tv-QPZwL]v-?_)4Cec","parent":"Fo3;}RZh889lf|v)*Wc8","inputs":{"VALUE":[1,[10,"2"]]},"fields":{"VARIABLE":["\bb-damage","|fg9Sax{3^M|yQ/@+Pb^-\bb-damage-"]},"shadow":false,"topLevel":false},"s7Tv-QPZwL]v-?_)4Cec":{"opcode":"control_forever","next":null,"parent":"s.no,^PDr@)KV@#Z([zR","inputs":{"SUBSTACK":[2,"7/?b1TW-/8UT)P}LqB|t"]},"fields":{},"shadow":false,"topLevel":false},"7/?b1TW-/8UT)P}LqB|t":{"opcode":"motion_turnright","next":"phnMvNrC#eBgG0L!TxRq","parent":"s7Tv-QPZwL]v-?_)4Cec","inputs":{"DEGREES":[3,[12,"\bb-damage","|fg9Sax{3^M|yQ/@+Pb^-\bb-damage-"],[4,10]]},"fields":{},"shadow":false,"topLevel":false},"phnMvNrC#eBgG0L!TxRq":{"opcode":"data_changevariableby","next":null,"parent":"7/?b1TW-/8UT)P}LqB|t","inputs":{"VALUE":[1,[4,0.1]]},"fields":{"VARIABLE":["\bb-damage","|fg9Sax{3^M|yQ/@+Pb^-\bb-damage-"]},"shadow":false,"topLevel":false}},"comments":{},"currentCostume":0,"costumes":[{"assetId":"bcaaa8547a07cfe572c0967ba829e99d","name":"costume1","bitmapResolution":1,"md5ext":"bcaaa8547a07cfe572c0967ba829e99d.svg","dataFormat":"svg","rotationCenterX":47,"rotationCenterY":55},{"assetId":"11d6c5fbd91e433a1b85a00fd9dd43b6","name":"costume2","bitmapResolution":1,"md5ext":"11d6c5fbd91e433a1b85a00fd9dd43b6.svg","dataFormat":"svg","rotationCenterX":47,"rotationCenterY":55}],"sounds":[{"assetId":"83c36d806dc92327b9e7049a565c6bff","name":"meow","dataFormat":"wav","format":"","rate":44100,"sampleCount":37376,"md5ext":"83c36d806dc92327b9e7049a565c6bff.wav"}],"volume":100,"layerOrder":1,"visible":true,"x":-12.550000000000011,"y":-22,"size":100,"direction":105.79999999757408,"draggable":false,"rotationStyle":"all around"}],"monitors":[{"id":"|fg9Sax{3^M|yQ/@+Pb^-\bb-damage-","mode":"default","opcode":"data_variable","params":{"VARIABLE":"\bb-damage"},"spriteName":null,"value":47.600000000000406,"x":5,"y":5,"visible":true,"sliderMin":0,"sliderMax":100,"isDiscrete":true}],"extensions":[],"meta":{"semver":"3.0.0","vm":"0.2.0-prerelease.20190213190040","agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.119 Safari/537.36"}}
37 changes: 37 additions & 0 deletions test/unit/parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
var fs = require('fs');
var path = require('path');
var test = require('tap').test;
var parse = require('../../lib/parse');
var data = require('../fixtures/data');
Expand All @@ -24,3 +26,38 @@ test('invalid', function (t) {
t.end();
});
});

test('backspace control characters get stripped out in sb2', function (t) {
var backspaceControlSb2 = fs.readFileSync(
path.resolve(__dirname, '../fixtures/data/217845144_sb2_control_char_in_strings_credit_to_Mr-Dave_test.json'));
parse(backspaceControlSb2.toString(), function (err, res) {
t.equal(err, null);
t.type(res, 'object');

// Verify that the object doesn't have a control character in the global variable name
t.equal(Array.isArray(res.variables), true);
t.equal(res.variables.length, 1);
// The parsed string should no longer include a backspace control character
t.equal(res.variables[0].name, 'b-damage');
t.end();
});
});

test('backspace control characters get stripped out in sb3', function (t) {
var backspaceControlSb2 = fs.readFileSync(
path.resolve(__dirname, '../fixtures/data/217845144_sb3_control_char_in_strings_credit_to_Mr-Dave_test.json'));
parse(backspaceControlSb2.toString(), function (err, res) {
t.equal(err, null);
t.type(res, 'object');

// Verify that the object doesn't have a control character in the global variable name
t.equal(Array.isArray(res.targets), true);
t.equal(res.targets.length, 2);
t.type(res.targets[0].variables, 'object');
var backspaceControlVariableId = Object.keys(res.targets[0].variables)[0];
var backspaceControlVariable = res.targets[0].variables[backspaceControlVariableId];
// The parsed string should no longer include a backspace control character
t.equal(backspaceControlVariable[0], 'b-damage');
t.end();
});
});