Skip to content
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

Safe mechanism for converting strings to existing atoms #200

Open
wants to merge 2 commits into
base: develop-3.0
Choose a base branch
from

Conversation

macintux
Copy link
Contributor

@macintux macintux commented Aug 4, 2015

Proposed alternative to @seancribbs's #181 (RIAK-1392) that doesn't place existing schemas at risk. Don't allow user input to explode the atom table.

@jadeallenx
Copy link

I am leaning more toward #200 than #181 (RIAK-1392) (RIAK-1392) on safety grounds but maybe there's compelling future work that #181 (RIAK-1392) (RIAK-1392) enables I'm missing. Thoughts @seancribbs?

@seancribbs
Copy link
Contributor

Biased opinion: #181 (RIAK-1392) is safer. If you're allowing a user to create atoms via your config file, you're going to have a bad time.

@seancribbs
Copy link
Contributor

Addendum: I don't know any case where #181 (RIAK-1392) would break existing schemas.

@macintux
Copy link
Contributor Author

macintux commented Aug 5, 2015

Realistically, though, how many atoms can you create in a config file (barring active attempt to break the system)?

I’m in favor of #181 (RIAK-1392) (RIAK-1392) if I can be convinced it’s harmless, just don’t know how to evaluate that risk for schemas we don’t know about.

-John

On Aug 5, 2015, at 3:54 PM, Sean Cribbs notifications@github.com wrote:

Biased opinion: #181 (RIAK-1392) (RIAK-1392) #181 is safer. If you're allowing a user to create atoms via your config file, you're going to have a bad time.


Reply to this email directly or view it on GitHub #200 (comment).

@seancribbs
Copy link
Contributor

It honestly doesn't really matter that much to me; I just don't see how existing_atom gets you any significant benefit. If there are multiple possible values, the schema writer should use enum anyway.

@martincox martincox changed the base branch from develop to develop-3.0 January 25, 2022 15:37
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.

3 participants