Constructor for Symbol class in VM does not validate - should this be declared correct?

[DartBot]

This issue was originally filed by @mhausner

---

The VM version of the const constructor Symbol() is wrong. It has a non-const initializer.

const Symbol(String name)
      : this._name = validate(name);

I am implementing stricter constness checks in the VM right now and am removing the call to validate(). This results in corelib/symbol_test failing.

[floitschG]

cc @peter-ahe-google.

[iposva-google]

cc @gbracha.

[gbracha]

I think that it would be quite reasonable for Symbol to encode any string. Symbol literals are syntactically restricted to (optionally qualified) identifiers but the class Symbol need not enforce that I think.

[peter-ahe-google]

The current specification of Symbol was agreed upon in the initial specification and is implemented as a special rule in dart2js. The VM should be able to do the same.

---

Removed Area-Library label.
Added Area-VM label.

[floitschG]

Issue #19636 has been merged into this issue.

[lrhn]

The current design requires special compiler recognition and handling of the Symbol constructor.

A const invocation of const Symbol(constantString) will need to check, at compile-time, that constantString is one of the allowed arguments, and then create a symbol value corresponding to it.
A non-const invocation, new Symbol(someString) will need to call a constructor that does validation of the argument, something that a const constructor cannot do.

It's current declaration signature is external const factory Symbol(String). That is a valid Dart declaration, so the libraries are valid (even if only technically so). The is no way the Symbol constructor can be implemented in plain Dart, it has to use compiler magic.

The VM's current approach is to just not implement the specified behavior. Apparently it hasn't been a problem (it does allow all the cases that should be allowed, it just allows more as well).

There are usability consequences, though. They cannot make it an error if someone tries to create a private symbol as const Symbol("_foo"), even though it will not create a private symbol and it is not equal to #_foo for any library (it is actually equal to the library name of a library with declaration library _foo;, so there is that). And, obviously, code written using the more lenient implementation won't work on other platforms.

[lrhn]

This is still an issue: The VM accepts arguments to the constructor which it is specified to reject.

On top of that, the VM assumes that a user provided symbol string does follow the specification, and mangles the user input.

var uri = Uri.parse("http://foo:bar@baz.qux/test.dyt");
var symbol = Symbol(uri.authority);  // Should be rejected.
print(symbol); // Prints Symbol("bar.qux").
print(#bar.qux); // Prints Symbol("bar.qux").
print(symbol == #bar.qux);  // false

This happens because the VM accepts unvalidated user inputs, but it also assumes that it can embed its own secret information in the string. Those two are choices are incompatible. The VM needs to either validate user inputs, or find a way to avoid that its internal representation clashes with user inputs (maybe insert escapes if the user input contains : or @, and remove those escapes in the unmangle method).

This is not a front-end issue, the issue happens with run-time created objects (the front end does check constants in the constant transformer, but apparently the VM doesn't use that).

(I'm personally OK with removing the need to validate symbol inputs, but then the VM still needs to avoid collisions with its internal string encodings).