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

node list GraphQL Api : add fields + Add GraphQL Api to fetch transactions list on type #51

Closed
wants to merge 3 commits into from

Conversation

redDwarf03
Copy link
Member

See Issue #49

resolve(fn args, _ ->
type = Map.get(args, :type)
page = Map.get(args, :page, 1)
{:ok, Resolver.network_transactions(String.to_atom(type), page)}
Copy link

Choose a reason for hiding this comment

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

You should avoid to use String.to_atom as it may create a overflow of the atom list in the Erlang VM. It's a security issue, and a way to bring down nodes for attackers.

But you can either create a function to parse stringified type to atomized type, or use String.to_existing_atom

resolve(fn args, _ ->
type = Map.get(args, :type)
page = Map.get(args, :page, 1)
{:ok, Resolver.network_transactions(String.to_atom(type), page)}
Copy link

Choose a reason for hiding this comment

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

You should avoid to use String.to_atom as it may create a overflow of the atom table in the Erlang VM. It's a security issue, and a way to bring down nodes for attackers.

But you can create a function to parse stringified type to atomized type. This approach will enable a validation for the existing type and will sanitize the input.

Those checks can be done in the resolver itself.

@ghost ghost added feature New feature request good first issue Good for newcomers labels Aug 30, 2021
@redDwarf03
Copy link
Member Author

is it possible to fix this PR with someone who have better skills in elixir than me please :) that's a small evolution ;)
thank you for that

@ghost ghost closed this Jan 17, 2022
@ghost
Copy link

ghost commented Jan 17, 2022

Moved to #198

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant