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

refactor(sql): refactor sql queries to use prepared statements #4305

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fernandoonrails
Copy link

@fernandoonrails fernandoonrails commented Jan 21, 2025

in this commit, a the sql queries for the resource domain in the database were refactored to use prepared statements instead of simple queries, as it usually, makes the project safer when exposed to public internet

…e resources

in this commit, a the sql queries for inserting resources into the
database were refactores to use prepared statements instead of simple
queries

this change will:
- [X] add more security to the database for common attack such as sql
injection
@fernandoonrails
Copy link
Author

will take a look at it later, any help or critic to improve it would be much appreciated 😁

stmt := "INSERT INTO `resource` (" + strings.Join(fields, ", ") + ") VALUES (" + strings.Join(placeholder, ", ") + ")"
result, err := d.db.ExecContext(ctx, stmt, args...)
query := "INSERT INTO `resource` (" + strings.Join(fields, ", ") + ") VALUES (" + strings.Join(placeholder, ", ") + ")"
stmt, err := d.db.PrepareContext(ctx, query)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these db.PrepareContext calls needed?

The way I understand the docs is that they are intended to wrap SQL statements and are added to some form of a SQL statement file from where they can be called and reused or passed around between functions.

The way they are used in this PR does not allow for reuse and looks to me like a step that can be omitted.

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.

2 participants