prototypejs / prototype

Prototype JavaScript framework
http://prototypejs.org/
Other
3.54k stars 639 forks source link

SyntaxError: Badly formed JSON string: '' #295

Closed santonica closed 9 years ago

santonica commented 9 years ago

https://github.com/sstephenson/prototype/issues/295#fullscreenI see all the latest versions have this issue and the code associated with is the evalJSON function. There seems to be a coding issue in closing the parenthesis for the catch clause. The throw should be inside the curly braces.

  evalJSON: function(sanitize) {
    var json = this.unfilterJSON();
    try {
      if (!sanitize || json.isJSON()) return eval('(' + json + ')');
    } catch (e) { }
     throw new SyntaxError('Badly formed JSON string: ' + this.inspect());
  },

Can someone validate?

The fix would be to place the curly braces correctly.

catch (e) { throw new SyntaxError('Badly formed JSON string: ' + this.inspect()); }

jwestbrook commented 9 years ago

Actually the code is correct as that method will not get to the throw unless the text is badly formed/invalid JSON.

Consider this line

if (!sanitize || json.isJSON()) return eval('(' + json + ')');

if the json object exists, and is JSON and no error is thrown then the method will return immediately. So if either an error or warning is thrown or json.isJSON() returns false and sanitize is true then a Syntax Error is thrown instead of only being thrown if an error/warning is thrown in the try block.

edit : I assume you mean curly brackets instead of parenthesis.

santonica commented 9 years ago

Thx Jwestbrook. I indeed meant curly braces. my bad..corrected. I see what you are saying and it will work fine. I was caught off guard by all the control statements thrown in. May be something like below would be more readable.

   evalJSON: function(sanitize) {
      var json = this.unfilterJSON();
      var _onErrorFn = function() {
          throw new SyntaxError('Badly formed JSON string: ' + this.inspect());
      };
      if (sanitize && !json.isJSON()) 
          _onErrorFn.call(this);
      try {
          return eval('(' + json + ')');
      } catch (e) {
          _onErrorFn.call(this);
      }
    }
savetheclocktower commented 9 years ago

I'll add a small clarifying comment, if only because I found this hard to read myself.