Clojure

Reader literals cannot contain periods

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: Release 1.7
  • Component/s: None
  • Labels:
  • Approval:
    Vetted

Description

The reader tries to read a record instead of a literal if the tag contains periods.

user> (binding [*data-readers* {'foo/bar #'identity}] (read-string "#foo/bar 1"))
1
user> (binding [*data-readers* {'foo/bar.x #'identity}] (read-string "#foo/bar.x 1"))
ClassNotFoundException foo/bar.x  java.lang.Class.forName0 (Class.java:-2)

Summary of reader forms:

Kind Example Constraint Status
Record #user.Foo[1] record class name OK
Class #java.lang.String["abc"] class name OK
Clojure reader tag #uuid "c48d7d6e-f3bb-425a-abc5-44bd014a511d" not a class name, no "/" OK
Library reader tag #my/card "5H" not a class name, has "/" OK
  #my.ns/card "5H" not a class name, has "/" OK
  #my/playing.card "5H" not a class name, has "/" BROKEN - read as record

Note: reader tags should not be allowed to override the record reader.

Cause: In LispReader, CtorReader.invoke() decides between record and tagged literal based on whether the tag has a ".".

Proposed: Change the discriminator in CtorReader.

Alternative 1 (purely string inspection):

  • If name has a "/" -> readTagged (not a legal class name)
  • If name has no "/" or "." -> readTagged (records must have qualified names)
  • Else -> readRecord (also covers Java classes)

Tradeoffs: Clojure-defined data reader tags must not contain periods. Not possible to read a Java class with no package. Avoids unnecessary class loading/construction for all tags.

Alternative 2 (prioritize Class check):

  • Attempt readRecord (also covers Java classes)
  • If failed, attempt readTagged

Tradeoffs: Clojure tags could not override Java/record constructors - I'm not sure that's something we'd ever want to do, but this would cut that off. This alternative may attempt classloading when it would not have before.

Hybrids of these are also possible.

Patch:

Screened by:

Activity

Kevin Lynagh made changes -
Field Original Value New Value
Description The LispReader tries to read a record instead of a literal if the tag contains periods:

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/LispReader.java#L1171

Which effectively means that reader tags cannot contain periods.
The EDN spec is unclear on this:

    edn supports extensibility through a simple mechanism. # followed immediately by a symbol starting with an alphabetic character indicates that that symbol is a tag.

If periods are allowed, then the LispReader should first check to see if the tag is in *data-readers* and only then if not try to initialize a Java class.

I'm happy to write the patch if this behavior is what is desired.
The LispReader tries to read a record instead of a literal if the tag contains periods:

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/LispReader.java#L1171

Which effectively means that reader tags cannot contain periods.
The EDN spec is unclear on this:

    edn supports extensibility through a simple mechanism. # followed immediately by a symbol starting with an alphabetic character indicates that that symbol is a tag.

(issue opened: https://github.com/edn-format/edn/issues/39)

If periods are allowed, then the LispReader should first check to see if the tag is in *data-readers* and only then if not try to initialize a Java class.

I'm happy to write the patch if this behavior is what is desired.
Kevin Lynagh made changes -
Description The LispReader tries to read a record instead of a literal if the tag contains periods:

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/LispReader.java#L1171

Which effectively means that reader tags cannot contain periods.
The EDN spec is unclear on this:

    edn supports extensibility through a simple mechanism. # followed immediately by a symbol starting with an alphabetic character indicates that that symbol is a tag.

(issue opened: https://github.com/edn-format/edn/issues/39)

If periods are allowed, then the LispReader should first check to see if the tag is in *data-readers* and only then if not try to initialize a Java class.

I'm happy to write the patch if this behavior is what is desired.
The LispReader tries to read a record instead of a literal if the tag contains periods:

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/LispReader.java#L1171

Which effectively means that reader tags cannot contain periods.
The EDN spec is unclear on this:

bq. edn supports extensibility through a simple mechanism. # followed immediately by a symbol starting with an alphabetic character indicates that that symbol is a tag.

(issue opened: https://github.com/edn-format/edn/issues/39)

If periods are allowed, then the LispReader should first check to see if the tag is in *data-readers* and only then if not try to initialize a Java class.

I'm happy to write the patch if this behavior is what is desired.
Kevin Lynagh made changes -
Description The LispReader tries to read a record instead of a literal if the tag contains periods:

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/LispReader.java#L1171

Which effectively means that reader tags cannot contain periods.
The EDN spec is unclear on this:

bq. edn supports extensibility through a simple mechanism. # followed immediately by a symbol starting with an alphabetic character indicates that that symbol is a tag.

(issue opened: https://github.com/edn-format/edn/issues/39)

If periods are allowed, then the LispReader should first check to see if the tag is in *data-readers* and only then if not try to initialize a Java class.

I'm happy to write the patch if this behavior is what is desired.
The LispReader tries to read a record instead of a literal if the tag contains periods:

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/LispReader.java#L1171

Which effectively means that reader tags cannot contain periods.
The EDN spec is unclear on this:

bq. edn supports extensibility through a simple mechanism. # followed immediately by a symbol starting with an alphabetic character indicates that that symbol is a tag.

(issue opened: https://github.com/edn-format/edn/issues/39)

If periods are allowed, then the LispReader should first check to see if the tag is in {{\*data-readers\*}} and only then if not try to initialize a Java class.

I'm happy to write the patch if this behavior is what is desired.
Rich Hickey made changes -
Fix Version/s Release 1.5 [ 10150 ]
Rich Hickey made changes -
Approval Vetted [ 10003 ]
Kevin Lynagh made changes -
Patch Code and Test [ 10002 ]
Attachment clj-1100-reader-literal-periods.patch [ 11665 ]
Steve Miner made changes -
Attachment CLJ-1100-allow-periods-in-data-reader-tags.patch [ 11666 ]
Steve Miner made changes -
Attachment CLJ-1100-allow-periods-in-data-reader-tags.patch [ 11666 ]
Steve Miner made changes -
Attachment CLJ-1100-periods-in-data-reader-tags.patch [ 11716 ]
Rich Hickey made changes -
Fix Version/s Release 1.5 [ 10150 ]
Steve Miner made changes -
Attachment CLJ-1100-periods-in-data-reader-tags.patch [ 11716 ]
Steve Miner made changes -
Attachment CLJ-1100-periods-in-data-reader-tags.patch [ 11731 ]
Steve Miner made changes -
Comment [ Allow periods in data-reader tags. Tries \*data-readers* first, then default-data-readers as usual. Then tries tag as a possible record class, and finally tries the \*default-data-reader-fn*. More flexible than assuming any tag with a period is a record class. Note, with this patch, \*data-readers* can override a record class. I'm calling that a feature. ]
Steve Miner made changes -
Comment [ Revised my patch to add more tests and be more discriminating when considering if a tag could a record class name. The previous comments still apply. ]
Andy Fingerhut made changes -
Attachment clj-1100-periods-in-data-reader-tags-patch-v2.txt [ 11842 ]
Andy Fingerhut made changes -
Attachment clj-1100-periods-in-data-reader-tags-patch-v2.txt [ 11842 ]
Steve Miner made changes -
Attachment CLJ-1100-periods-in-data-reader-tags.patch [ 11731 ]
Steve Miner made changes -
Patch Code and Test [ 10002 ]
Kevin Lynagh made changes -
Attachment clj-1100-reader-literal-periods.patch [ 11665 ]
Alex Miller made changes -
Description The LispReader tries to read a record instead of a literal if the tag contains periods:

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/LispReader.java#L1171

Which effectively means that reader tags cannot contain periods.
The EDN spec is unclear on this:

bq. edn supports extensibility through a simple mechanism. # followed immediately by a symbol starting with an alphabetic character indicates that that symbol is a tag.

(issue opened: https://github.com/edn-format/edn/issues/39)

If periods are allowed, then the LispReader should first check to see if the tag is in {{\*data-readers\*}} and only then if not try to initialize a Java class.

I'm happy to write the patch if this behavior is what is desired.
The reader tries to read a record instead of a literal if the tag contains periods.

{code}
user> (binding [*data-readers* {'foo/bar #'identity}] (read-string "#foo/bar 1"))
1
user> (binding [*data-readers* {'foo/bar.x #'identity}] (read-string "#foo/bar.x 1"))
ClassNotFoundException foo/bar.x java.lang.Class.forName0 (Class.java:-2)
{code}

Summary of reader forms:

|Kind|Example|Constraint|Status
|Record|#user.Foo[1]|no "/", dotted name|OK
|Class|#java.lang.String["abc"]|no "/", dotted name|OK
|Clojure reader tag|#uuid "c48d7d6e-f3bb-425a-abc5-44bd014a511d"|no "/", not a class name|OK
|Library reader tag|#my/card "5H"|has "/"|OK
||#my.ns/card "5H"|has "/"|OK
||#my/playing.card "5H"|has "/"|BROKEN - read as record

Note: reader tags should not be allowed to override the record reader.

*Cause:* In LispReader, CtorReader.invoke() decides between record and tagged literal based on whether the tag has a ".".

*Proposed:* Change the discriminator in CtorReader.

Alternative 1:
- If name has a "/" -> readTagged (not a legal class name)
- If name has no "/" or "." -> readTagged (records must have qualified names)
- Else -> readRecord (also covers Java classes)

Tradeoffs: Clojure-defined data reader tags must not contain periods. Not possible to read a Java class with no package. Avoids unnecessary class loading/construction for all tags.

Alternative 2:
- Attempt readRecord (also covers Java classes)
- If failed, attempt readTagged

Tradeoffs: Clojure tags could not override Java/record constructors - I'm not sure that's something we'd ever want to do, but this would cut that off. This alternative may attempt classloading when it would not have before.

Hybrids of these are also possible.

*Patch:*

*Screened by:*
Alex Miller made changes -
Description The reader tries to read a record instead of a literal if the tag contains periods.

{code}
user> (binding [*data-readers* {'foo/bar #'identity}] (read-string "#foo/bar 1"))
1
user> (binding [*data-readers* {'foo/bar.x #'identity}] (read-string "#foo/bar.x 1"))
ClassNotFoundException foo/bar.x java.lang.Class.forName0 (Class.java:-2)
{code}

Summary of reader forms:

|Kind|Example|Constraint|Status
|Record|#user.Foo[1]|no "/", dotted name|OK
|Class|#java.lang.String["abc"]|no "/", dotted name|OK
|Clojure reader tag|#uuid "c48d7d6e-f3bb-425a-abc5-44bd014a511d"|no "/", not a class name|OK
|Library reader tag|#my/card "5H"|has "/"|OK
||#my.ns/card "5H"|has "/"|OK
||#my/playing.card "5H"|has "/"|BROKEN - read as record

Note: reader tags should not be allowed to override the record reader.

*Cause:* In LispReader, CtorReader.invoke() decides between record and tagged literal based on whether the tag has a ".".

*Proposed:* Change the discriminator in CtorReader.

Alternative 1:
- If name has a "/" -> readTagged (not a legal class name)
- If name has no "/" or "." -> readTagged (records must have qualified names)
- Else -> readRecord (also covers Java classes)

Tradeoffs: Clojure-defined data reader tags must not contain periods. Not possible to read a Java class with no package. Avoids unnecessary class loading/construction for all tags.

Alternative 2:
- Attempt readRecord (also covers Java classes)
- If failed, attempt readTagged

Tradeoffs: Clojure tags could not override Java/record constructors - I'm not sure that's something we'd ever want to do, but this would cut that off. This alternative may attempt classloading when it would not have before.

Hybrids of these are also possible.

*Patch:*

*Screened by:*
The reader tries to read a record instead of a literal if the tag contains periods.

{code}
user> (binding [*data-readers* {'foo/bar #'identity}] (read-string "#foo/bar 1"))
1
user> (binding [*data-readers* {'foo/bar.x #'identity}] (read-string "#foo/bar.x 1"))
ClassNotFoundException foo/bar.x java.lang.Class.forName0 (Class.java:-2)
{code}

Summary of reader forms:

|Kind|Example|Constraint|Status
|Record|#user.Foo[1]|no "/", dotted name|OK
|Class|#java.lang.String["abc"]|no "/", dotted name|OK
|Clojure reader tag|#uuid "c48d7d6e-f3bb-425a-abc5-44bd014a511d"|no "/", not a class name|OK
|Library reader tag|#my/card "5H"|has "/"|OK
| |#my.ns/card "5H"|has "/"|OK
| |#my/playing.card "5H"|has "/"|BROKEN - read as record

Note: reader tags should not be allowed to override the record reader.

*Cause:* In LispReader, CtorReader.invoke() decides between record and tagged literal based on whether the tag has a ".".

*Proposed:* Change the discriminator in CtorReader.

Alternative 1:
- If name has a "/" -> readTagged (not a legal class name)
- If name has no "/" or "." -> readTagged (records must have qualified names)
- Else -> readRecord (also covers Java classes)

Tradeoffs: Clojure-defined data reader tags must not contain periods. Not possible to read a Java class with no package. Avoids unnecessary class loading/construction for all tags.

Alternative 2:
- Attempt readRecord (also covers Java classes)
- If failed, attempt readTagged

Tradeoffs: Clojure tags could not override Java/record constructors - I'm not sure that's something we'd ever want to do, but this would cut that off. This alternative may attempt classloading when it would not have before.

Hybrids of these are also possible.

*Patch:*

*Screened by:*
Alex Miller made changes -
Description The reader tries to read a record instead of a literal if the tag contains periods.

{code}
user> (binding [*data-readers* {'foo/bar #'identity}] (read-string "#foo/bar 1"))
1
user> (binding [*data-readers* {'foo/bar.x #'identity}] (read-string "#foo/bar.x 1"))
ClassNotFoundException foo/bar.x java.lang.Class.forName0 (Class.java:-2)
{code}

Summary of reader forms:

|Kind|Example|Constraint|Status
|Record|#user.Foo[1]|no "/", dotted name|OK
|Class|#java.lang.String["abc"]|no "/", dotted name|OK
|Clojure reader tag|#uuid "c48d7d6e-f3bb-425a-abc5-44bd014a511d"|no "/", not a class name|OK
|Library reader tag|#my/card "5H"|has "/"|OK
| |#my.ns/card "5H"|has "/"|OK
| |#my/playing.card "5H"|has "/"|BROKEN - read as record

Note: reader tags should not be allowed to override the record reader.

*Cause:* In LispReader, CtorReader.invoke() decides between record and tagged literal based on whether the tag has a ".".

*Proposed:* Change the discriminator in CtorReader.

Alternative 1:
- If name has a "/" -> readTagged (not a legal class name)
- If name has no "/" or "." -> readTagged (records must have qualified names)
- Else -> readRecord (also covers Java classes)

Tradeoffs: Clojure-defined data reader tags must not contain periods. Not possible to read a Java class with no package. Avoids unnecessary class loading/construction for all tags.

Alternative 2:
- Attempt readRecord (also covers Java classes)
- If failed, attempt readTagged

Tradeoffs: Clojure tags could not override Java/record constructors - I'm not sure that's something we'd ever want to do, but this would cut that off. This alternative may attempt classloading when it would not have before.

Hybrids of these are also possible.

*Patch:*

*Screened by:*
The reader tries to read a record instead of a literal if the tag contains periods.

{code}
user> (binding [*data-readers* {'foo/bar #'identity}] (read-string "#foo/bar 1"))
1
user> (binding [*data-readers* {'foo/bar.x #'identity}] (read-string "#foo/bar.x 1"))
ClassNotFoundException foo/bar.x java.lang.Class.forName0 (Class.java:-2)
{code}

Summary of reader forms:

|Kind|Example|Constraint|Status
|Record|#user.Foo[1]|record class name|OK
|Class|#java.lang.String["abc"]|class name|OK
|Clojure reader tag|#uuid "c48d7d6e-f3bb-425a-abc5-44bd014a511d"|not a class name, no "/"|OK
|Library reader tag|#my/card "5H"|not a class name, has "/"|OK
| |#my.ns/card "5H"|not a class name, has "/"|OK
| |#my/playing.card "5H"|not a class name, has "/"|BROKEN - read as record

Note: reader tags should not be allowed to override the record reader.

*Cause:* In LispReader, CtorReader.invoke() decides between record and tagged literal based on whether the tag has a ".".

*Proposed:* Change the discriminator in CtorReader.

Alternative 1:
- If name has a "/" -> readTagged (not a legal class name)
- If name has no "/" or "." -> readTagged (records must have qualified names)
- Else -> readRecord (also covers Java classes)

Tradeoffs: Clojure-defined data reader tags must not contain periods. Not possible to read a Java class with no package. Avoids unnecessary class loading/construction for all tags.

Alternative 2:
- Attempt readRecord (also covers Java classes)
- If failed, attempt readTagged

Tradeoffs: Clojure tags could not override Java/record constructors - I'm not sure that's something we'd ever want to do, but this would cut that off. This alternative may attempt classloading when it would not have before.

Hybrids of these are also possible.

*Patch:*

*Screened by:*
Alex Miller made changes -
Description The reader tries to read a record instead of a literal if the tag contains periods.

{code}
user> (binding [*data-readers* {'foo/bar #'identity}] (read-string "#foo/bar 1"))
1
user> (binding [*data-readers* {'foo/bar.x #'identity}] (read-string "#foo/bar.x 1"))
ClassNotFoundException foo/bar.x java.lang.Class.forName0 (Class.java:-2)
{code}

Summary of reader forms:

|Kind|Example|Constraint|Status
|Record|#user.Foo[1]|record class name|OK
|Class|#java.lang.String["abc"]|class name|OK
|Clojure reader tag|#uuid "c48d7d6e-f3bb-425a-abc5-44bd014a511d"|not a class name, no "/"|OK
|Library reader tag|#my/card "5H"|not a class name, has "/"|OK
| |#my.ns/card "5H"|not a class name, has "/"|OK
| |#my/playing.card "5H"|not a class name, has "/"|BROKEN - read as record

Note: reader tags should not be allowed to override the record reader.

*Cause:* In LispReader, CtorReader.invoke() decides between record and tagged literal based on whether the tag has a ".".

*Proposed:* Change the discriminator in CtorReader.

Alternative 1:
- If name has a "/" -> readTagged (not a legal class name)
- If name has no "/" or "." -> readTagged (records must have qualified names)
- Else -> readRecord (also covers Java classes)

Tradeoffs: Clojure-defined data reader tags must not contain periods. Not possible to read a Java class with no package. Avoids unnecessary class loading/construction for all tags.

Alternative 2:
- Attempt readRecord (also covers Java classes)
- If failed, attempt readTagged

Tradeoffs: Clojure tags could not override Java/record constructors - I'm not sure that's something we'd ever want to do, but this would cut that off. This alternative may attempt classloading when it would not have before.

Hybrids of these are also possible.

*Patch:*

*Screened by:*
The reader tries to read a record instead of a literal if the tag contains periods.

{code}
user> (binding [*data-readers* {'foo/bar #'identity}] (read-string "#foo/bar 1"))
1
user> (binding [*data-readers* {'foo/bar.x #'identity}] (read-string "#foo/bar.x 1"))
ClassNotFoundException foo/bar.x java.lang.Class.forName0 (Class.java:-2)
{code}

Summary of reader forms:

|Kind|Example|Constraint|Status
|Record|#user.Foo[1]|record class name|OK
|Class|#java.lang.String["abc"]|class name|OK
|Clojure reader tag|#uuid "c48d7d6e-f3bb-425a-abc5-44bd014a511d"|not a class name, no "/"|OK
|Library reader tag|#my/card "5H"|not a class name, has "/"|OK
| |#my.ns/card "5H"|not a class name, has "/"|OK
| |#my/playing.card "5H"|not a class name, has "/"|BROKEN - read as record

Note: reader tags should not be allowed to override the record reader.

*Cause:* In LispReader, CtorReader.invoke() decides between record and tagged literal based on whether the tag has a ".".

*Proposed:* Change the discriminator in CtorReader.

Alternative 1 (purely string inspection):
- If name has a "/" -> readTagged (not a legal class name)
- If name has no "/" or "." -> readTagged (records must have qualified names)
- Else -> readRecord (also covers Java classes)

Tradeoffs: Clojure-defined data reader tags must not contain periods. Not possible to read a Java class with no package. Avoids unnecessary class loading/construction for all tags.

Alternative 2 (prioritize Class check):
- Attempt readRecord (also covers Java classes)
- If failed, attempt readTagged

Tradeoffs: Clojure tags could not override Java/record constructors - I'm not sure that's something we'd ever want to do, but this would cut that off. This alternative may attempt classloading when it would not have before.

Hybrids of these are also possible.

*Patch:*

*Screened by:*
Alex Miller made changes -
Priority Major [ 3 ] Minor [ 4 ]
Rich Hickey made changes -
Fix Version/s Release 1.7 [ 10250 ]

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated: