Closed Bug 73598 Opened 24 years ago Closed 24 years ago

Error in toSource() where a property name is a reserved word

Categories

(Core :: JavaScript Engine, defect, P3)

x86
All
defect

Tracking

()

RESOLVED FIXED
mozilla0.9

People

(Reporter: petew, Assigned: brendan)

Details

(Keywords: js1.5)

Attachments

(1 file)

var x = new Object; x["new"] = "a value"; var serial = x.toSource(); This returns serial as ({new:'a value'}) eval() of this fails because 'new' is an illegal identifier. Suggested fix: in the toSource code (jsobj.c#692 in RC3) already quotes property names that contain illegal property characters. Can either change this to check for reserved words, or simply quote everything. I simply remove the test for illegal identifier characters and quote everything - this works fine!
I'm puzzled by this request. You're correct: "new" is a reserved word according to ECMA Edition 3, Section 7.5.2. And in Section 7.5.1 it says, "Reserved words cannot be used as identifiers" cc'ing Brendan for his opinion this -
My over-aesthetic code prefers unquoted identifiers, but the reporter is right: it can't get away without quotes for reserved identifiers. Patch coming up. /be
Assignee: rogerl → brendan
Status: UNCONFIRMED → NEW
Ever confirmed: true
Status: NEW → ASSIGNED
Keywords: js1.5, mozilla0.9
Priority: -- → P3
Target Milestone: --- → mozilla0.9
Brenden - I've posted a more complete description on the netscape.public.mozilla.jseng board. I've included a more detailed description below, but in summary - 'toSource' generates an identifier in its output even if the property name is a reserved word - in fact the object literal syntax (ECMA-262, 3rd edition Dec 1999, section 11.1.5) allows the PropertyName of a PropertyNameAndValueList to be one of: Identifier number string These three options are correctly interpreted by the compiler and by 'eval()' The problem is that 'toSource' uses the string variant _only_ if the property name only contains illegal characters - which does _not_ cover all cases of illegal identifiers. A reserved word is an illegal identifier. In these cases toSource should use the string variant. Any legal JS object should be capable of being serialised using toSource and recovered using eval - they should be inverse operations to be useful. ------------------------------------------------------------------- --------------- Detail Below! Skip if I've said enough! ----------- Basically - I can create a property with any name I want using the associative access - i.e. var x - new Object x["default"] = "Hello World" x["%^&*&"] = "Goodbye World" In neither case is the quoted string a legal identifier - first case because it is a reserved word, second because it contains illegal characters. It does however work as I would expect - it would be a weekness of associative arrays if this were not the case. If I serialise the above example using 'toSource()' I get: ({default:'Hello World', '%^&*&':'Goodbye World'}) Note that the illegal identifier - because of characters - has been quoted - there is explicit code in jsobj.c to test for this and do it. You can test this simply enough by executing: var anObj = {'%^&*&':'value'} Then iterating over the contents. Because the name is quoted it is not an identifier but a string. 'new' is not a legal identifier, even though it contains only legal identifier characters - but is a legal associative index. So: var anObj = {new:'value'} fails but var anObj = {'new':'value'} works. At the very least I'd expect 'eval' to be able to recreate anything 'toSource' can produce. I don Hope that helps.
petew: I understood the problem right away, no need to repeat. I should point out that SpiderMonkey supports uneval as a complete inverse of eval, which works for primitive types as well as for objects (unlike toSource). /be
Thanks Brenden - I wasn't aware of uneval() - which is going to make my life a lot easier - thanks! Apologies for all the detail - I was writing it before you posted your reply. Pete
Attached patch proposed fix (deleted) — Splinter Review
This one's easy. Please r= and sr= ASAP. /be
Keywords: patch, review
r/sr=jband
Fixed. /be
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: