Skip to content

feat(pass-style): add selector#2774

Closed
kumavis wants to merge 1 commit intomasterfrom
kumavis/pass-style-selector
Closed

feat(pass-style): add selector#2774
kumavis wants to merge 1 commit intomasterfrom
kumavis/pass-style-selector

Conversation

@kumavis
Copy link
Copy Markdown
Member

@kumavis kumavis commented Apr 25, 2025

No description provided.

@mhofman
Copy link
Copy Markdown
Contributor

mhofman commented Apr 25, 2025

What's a selector?

@kriskowal
Copy link
Copy Markdown
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
Copy Markdown
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.

| 'symbol';

export type ContainerStyle = 'copyRecord' | 'copyArray' | 'tagged';
export type ContainerStyle = 'copyRecord' | 'copyArray' | 'tagged' | 'selector';
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

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).

| 'promise';

export type TaggedOrRemotable = 'tagged' | 'remotable';
export type TaggedOrRemotable = 'tagged' | 'selector' | 'remotable';
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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