Skip to content

feat(pass-style): add selector #2774

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.

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

Closed
wants to merge 1 commit into from
Closed

Conversation

kumavis
Copy link
Member

@kumavis kumavis commented Apr 25, 2025

No description provided.

@mhofman
Copy link
Contributor

mhofman commented Apr 25, 2025

What's a selector?

@kriskowal
Copy link
Member

What's a selector?

OCapN Selector corresponds to Syrup symbol and Scheme symbol. Because we can’t use JavaScript symbols (risk of registry stuffing attacks), MarkM insisted we give this a distinguishing name. It follows that it should have its own passStyle, which should replace symbols entirely in the long run.

https://github.com/ocapn/ocapn/blob/main/draft-specifications/Model.md#selector

Copy link
Member

@kriskowal kriskowal left a comment

Choose a reason for hiding this comment

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

I defer to @erights and @mhofman in this package. I have questions about whether Selector (née Symbol) fits in the established categories. It represents an Atom on the wire, but one we are obliged to reïfy as an Object.

@@ -22,7 +22,7 @@ export type PrimitiveStyle =
| 'string'
| 'symbol';

export type ContainerStyle = 'copyRecord' | 'copyArray' | 'tagged';
export type ContainerStyle = 'copyRecord' | 'copyArray' | 'tagged' | 'selector';
Copy link
Member

Choose a reason for hiding this comment

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

I don’t believe selector qualifies as a container. It’s more like a string. But, the reïfication of a selector is an object, so that might be the distinguishing feature for containers. I do not know.

Copy link
Member Author

Choose a reason for hiding this comment

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

its a container for a string! yeah ok was mostly just following 'tagged' around the code base

Copy link
Contributor

Choose a reason for hiding this comment

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

It's definitely not a container. Our best documentation is at https://endojs.github.io/endo/types/_endo_pass_style.Passable.html , and we could describe Selector as primitive if we're taking a data model perspective, or otherwise a new special case (because, unlike the existing primitives, Object.is will differentiate identical values).

@@ -31,7 +31,7 @@ export type PassStyle =
| 'error'
| 'promise';

export type TaggedOrRemotable = 'tagged' | 'remotable';
export type TaggedOrRemotable = 'tagged' | 'selector' | 'remotable';
Copy link
Member

Choose a reason for hiding this comment

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

Selector definitely does not belong in this union.

@kumavis
Copy link
Member Author

kumavis commented Apr 25, 2025

we can’t use JavaScript symbols (risk of registry stuffing attacks)

is the concern here only from using Symbol.for? would making our own symbol registry as a WeakRefMap (map to weakly held values) resolve the attack vector?

here is an example showing a Symbol being GC'd in nodejs

❯ node --expose-gc
Welcome to Node.js v20.18.0.
Type ".help" for more information.
> const registry = new FinalizationRegistry((heldValue) => console.log(`Object with value '${heldValue}' has been garbage collected.`));
undefined
> registry.register(Symbol('foo'), 'foo');
undefined
> gc()
undefined
> Object with value 'foo' has been garbage collected.

@erights
Copy link
Contributor

erights commented Apr 25, 2025

Seeing this thread, I realize I gave you bad advise. Selector is like Tagged in some ways, but is like Symbol in other ways. I'll make a PR staged on this one as my corrected review suggestions. Hold on.

would making our own symbol registry as a WeakRefMap (map to weakly held values) resolve the attack vector?

Would be an interesting experiment, but I don't expect it to be a better solution than what this PR is attempting.

@erights
Copy link
Contributor

erights commented Apr 26, 2025

#2777

Thanks for the great idea, and sorry for how long it took for me to get it.

@kumavis
Copy link
Member Author

kumavis commented Apr 28, 2025

closed for #2777

@kumavis kumavis closed this Apr 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants