Skip to content

Commit

Permalink
refactor(sql): refactor sql queries to use prepared statements for th…
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@fernandoonrails
fernandoonrails committed Jan 21, 2025
1 parent 702c092 commit db4f141
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 23 deletions.
50 changes: 42 additions & 8 deletions store/db/mysql/resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"database/sql"
"fmt"
"log/slog"
"strings"

"github.com/pkg/errors"
Expand All @@ -30,8 +31,13 @@ func (d *DB) CreateResource(ctx context.Context, create *store.Resource) (*store
}
args := []any{create.UID, create.Filename, create.Blob, create.Type, create.Size, create.CreatorID, create.MemoID, storageType, create.Reference, payloadString}

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)
if err != nil {
return nil, err
}
defer stmt.Close()
result, err := stmt.ExecContext(ctx, args...)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -86,11 +92,21 @@ func (d *DB) ListResources(ctx context.Context, find *store.FindResource) ([]*st
}
}

rows, err := d.db.QueryContext(ctx, query, args...)
stmt, err := d.db.PrepareContext(ctx, query)
if err != nil {
return nil, err
}
defer rows.Close()

rows, err := stmt.QueryContext(ctx, args...)

Check failure on line 100 in store/db/mysql/resource.go

View workflow job for this annotation

GitHub Actions / go-static-checks 

Rows/Stmt/NamedStmt was not closed (sqlclosecheck)

Check failure on line 100 in store/db/mysql/resource.go

View workflow job for this annotation

GitHub Actions / go-static-checks 

Rows/Stmt/NamedStmt was not closed (sqlclosecheck)
if err != nil {
return nil, err
}
defer func() {
err := stmt.Close()
if err != nil {
slog.Error("error closing statement", slog.String("message", err.Error()))
}
}()

list := make([]*store.Resource, 0)
for rows.Next() {
Expand Down Expand Up @@ -138,6 +154,7 @@ func (d *DB) ListResources(ctx context.Context, find *store.FindResource) ([]*st
return list, nil
}

// TODO! write a function that doesn't query everything, and instead, just one

Check failure on line 157 in store/db/mysql/resource.go

View workflow job for this annotation

GitHub Actions / go-static-checks 

Comment should end in a period (godot)

Check failure on line 157 in store/db/mysql/resource.go

View workflow job for this annotation

GitHub Actions / go-static-checks 

Comment should end in a period (godot)
func (d *DB) GetResource(ctx context.Context, find *store.FindResource) (*store.Resource, error) {
list, err := d.ListResources(ctx, find)
if err != nil {
Expand Down Expand Up @@ -177,8 +194,20 @@ func (d *DB) UpdateResource(ctx context.Context, update *store.UpdateResource) e
}

args = append(args, update.ID)
stmt := "UPDATE `resource` SET " + strings.Join(set, ", ") + " WHERE `id` = ?"
result, err := d.db.ExecContext(ctx, stmt, args...)
query := "UPDATE `resource` SET " + strings.Join(set, ", ") + " WHERE `id` = ?"
stmt, err := d.db.PrepareContext(ctx, query)
if err != nil {
return err
}

defer func() {
if err := stmt.Close(); err != nil {
slog.Error("error closing statement",
slog.String("message", err.Error()),
)
}
}()
result, err := stmt.ExecContext(ctx, args...)
if err != nil {
return err
}
Expand All @@ -189,8 +218,13 @@ func (d *DB) UpdateResource(ctx context.Context, update *store.UpdateResource) e
}

func (d *DB) DeleteResource(ctx context.Context, delete *store.DeleteResource) error {
stmt := "DELETE FROM `resource` WHERE `id` = ?"
result, err := d.db.ExecContext(ctx, stmt, delete.ID)
query := "DELETE FROM `resource` WHERE `id` = ?"
stmt, err := d.db.PrepareContext(ctx, query)
if err != nil {
return err
}
defer stmt.Close()
result, err := stmt.ExecContext(ctx, delete.ID)
if err != nil {
return err
}
Expand Down
71 changes: 63 additions & 8 deletions store/db/postgres/resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"database/sql"
"fmt"
"log/slog"
"strings"

"github.com/pkg/errors"
Expand All @@ -29,8 +30,20 @@ func (d *DB) CreateResource(ctx context.Context, create *store.Resource) (*store
}
args := []any{create.UID, create.Filename, create.Blob, create.Type, create.Size, create.CreatorID, create.MemoID, storageType, create.Reference, payloadString}

stmt := "INSERT INTO resource (" + strings.Join(fields, ", ") + ") VALUES (" + placeholders(len(args)) + ") RETURNING id, created_ts, updated_ts"
if err := d.db.QueryRowContext(ctx, stmt, args...).Scan(&create.ID, &create.CreatedTs, &create.UpdatedTs); err != nil {
query := "INSERT INTO resource (" + strings.Join(fields, ", ") + ") VALUES (" + placeholders(len(args)) + ") RETURNING id, created_ts, updated_ts"
stmt, err := d.db.PrepareContext(ctx, query)
if err != nil {
return nil, err
}
defer func() {
err := stmt.Close()
if err != nil {
slog.Error("failed to close statement",
slog.String("message", err.Error()),
)
}
}()
if err = stmt.QueryRowContext(ctx, args...).Scan(&create.ID, &create.CreatedTs, &create.UpdatedTs); err != nil {
return nil, err
}
return create, nil
Expand Down Expand Up @@ -82,12 +95,29 @@ func (d *DB) ListResources(ctx context.Context, find *store.FindResource) ([]*st
query = fmt.Sprintf("%s OFFSET %d", query, *find.Offset)
}
}
stmt, err := d.db.PrepareContext(ctx, query)
if err != nil {
return nil, err
}

rows, err := d.db.QueryContext(ctx, query, args...)
rows, err := stmt.QueryContext(ctx, args...)
if err != nil {
return nil, err
}
defer rows.Close()
defer func() {
err := stmt.Close()
if err != nil {
slog.Error("error closing statement",
slog.String("message", err.Error()),
)
}
err = rows.Close()
if err != nil {
slog.Error("error closing rows",
slog.String("message", err.Error()),
)
}
}()

list := make([]*store.Resource, 0)
for rows.Next() {
Expand Down Expand Up @@ -161,9 +191,21 @@ func (d *DB) UpdateResource(ctx context.Context, update *store.UpdateResource) e
set, args = append(set, "payload = "+placeholder(len(args)+1)), append(args, string(bytes))
}

stmt := `UPDATE resource SET ` + strings.Join(set, ", ") + ` WHERE id = ` + placeholder(len(args)+1)
query := `UPDATE resource SET ` + strings.Join(set, ", ") + ` WHERE id = ` + placeholder(len(args)+1)
stmt, err := d.db.PrepareContext(ctx, query)
if err != nil {
return err
}
defer func() {
err := stmt.Close()
if err != nil {
slog.Error("error closing statement",
slog.String("message", err.Error()),
)
}
}()
args = append(args, update.ID)
result, err := d.db.ExecContext(ctx, stmt, args...)
result, err := stmt.ExecContext(ctx, args...)
if err != nil {
return err
}
Expand All @@ -174,8 +216,21 @@ func (d *DB) UpdateResource(ctx context.Context, update *store.UpdateResource) e
}

func (d *DB) DeleteResource(ctx context.Context, delete *store.DeleteResource) error {
stmt := `DELETE FROM resource WHERE id = $1`
result, err := d.db.ExecContext(ctx, stmt, delete.ID)
query := `DELETE FROM resource WHERE id = $1`
stmt, err := d.db.PrepareContext(ctx, query)
if err != nil {
return err
}
defer func() {
err := stmt.Close()
if err != nil {
slog.Error("error closing statement",
slog.String("message", err.Error()),
)
}
}()

result, err := stmt.ExecContext(ctx, delete.ID)
if err != nil {
return err
}
Expand Down
58 changes: 51 additions & 7 deletions store/db/sqlite/resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"database/sql"
"fmt"
"log/slog"
"strings"

"github.com/pkg/errors"
Expand All @@ -30,8 +31,12 @@ func (d *DB) CreateResource(ctx context.Context, create *store.Resource) (*store
}
args := []any{create.UID, create.Filename, create.Blob, create.Type, create.Size, create.CreatorID, create.MemoID, storageType, create.Reference, payloadString}

stmt := "INSERT INTO `resource` (" + strings.Join(fields, ", ") + ") VALUES (" + strings.Join(placeholder, ", ") + ") RETURNING `id`, `created_ts`, `updated_ts`"
if err := d.db.QueryRowContext(ctx, stmt, args...).Scan(&create.ID, &create.CreatedTs, &create.UpdatedTs); err != nil {
query := "INSERT INTO `resource` (" + strings.Join(fields, ", ") + ") VALUES (" + strings.Join(placeholder, ", ") + ") RETURNING `id`, `created_ts`, `updated_ts`"
stmt, err := d.db.PrepareContext(ctx, query)

Check failure on line 35 in store/db/sqlite/resource.go

View workflow job for this annotation

GitHub Actions / go-static-checks 

Rows/Stmt/NamedStmt was not closed (sqlclosecheck)

Check failure on line 35 in store/db/sqlite/resource.go

View workflow job for this annotation

GitHub Actions / go-static-checks 

Rows/Stmt/NamedStmt was not closed (sqlclosecheck)
if err != nil {
return nil, err
}
if err := stmt.QueryRowContext(ctx, args...).Scan(&create.ID, &create.CreatedTs, &create.UpdatedTs); err != nil {
return nil, err
}

Expand Down Expand Up @@ -78,12 +83,30 @@ func (d *DB) ListResources(ctx context.Context, find *store.FindResource) ([]*st
query = fmt.Sprintf("%s OFFSET %d", query, *find.Offset)
}
}
stmt, err := d.db.PrepareContext(ctx, query)
if err != nil {
return nil, err
}

rows, err := d.db.QueryContext(ctx, query, args...)
rows, err := stmt.QueryContext(ctx, args...)
if err != nil {
return nil, err
}
defer rows.Close()
defer func() {
err := stmt.Close()
if err != nil {
slog.Error("failed to close statement",
slog.String("message", err.Error()),
)
}
err = rows.Close()
if err != nil {
slog.Error("failed to close rows",
slog.String("message", err.Error()),
)
}
}()

list := make([]*store.Resource, 0)
for rows.Next() {
Expand Down Expand Up @@ -158,8 +181,18 @@ func (d *DB) UpdateResource(ctx context.Context, update *store.UpdateResource) e
}

args = append(args, update.ID)
stmt := "UPDATE `resource` SET " + strings.Join(set, ", ") + " WHERE `id` = ?"
result, err := d.db.ExecContext(ctx, stmt, args...)
query := "UPDATE `resource` SET " + strings.Join(set, ", ") + " WHERE `id` = ?"
stmt, err := d.db.PrepareContext(ctx, query)
if err != nil {
return errors.Wrap(err, "failed to prepare statement")
}
defer func() {
if err := stmt.Close(); err != nil {
slog.Error("failed to close statement",
slog.String("message", err.Error()))
}
}()
result, err := stmt.ExecContext(ctx, args...)
if err != nil {
return errors.Wrap(err, "failed to update resource")
}
Expand All @@ -170,8 +203,19 @@ func (d *DB) UpdateResource(ctx context.Context, update *store.UpdateResource) e
}

func (d *DB) DeleteResource(ctx context.Context, delete *store.DeleteResource) error {
stmt := "DELETE FROM `resource` WHERE `id` = ?"
result, err := d.db.ExecContext(ctx, stmt, delete.ID)
query := "DELETE FROM `resource` WHERE `id` = ?"
stmt, err := d.db.PrepareContext(ctx, query)
if err != nil {
return err
}
defer func() {
if err := stmt.Close(); err != nil {
slog.Error("failed to close statement",
slog.String("message", err.Error()),
)
}
}()
result, err := stmt.ExecContext(ctx, stmt, delete.ID)
if err != nil {
return err
}
Expand Down

0 comments on commit db4f141

Please sign in to comment.