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

Use RawSQL instead of Create to be able to upgrade to gorm new version #220

Closed
wants to merge 2 commits into from

Conversation

magellancl
Copy link

#217

Making a PR to support the latest gorm upgrade because the gorm fix doesn't seem to be coming anytime soon.

I did this quickly, but I am a bit wary of potential SQL injection, any review is welcome.

@magellancl magellancl changed the title Use RawSQL instead of Create to be able to use gorm new version Use RawSQL instead of Create to be able to upgrade to gorm new version Mar 18, 2024
@@ -458,8 +458,7 @@ func (g *Gormigrate) unknownMigrationsHaveHappened() (bool, error) {
}

func (g *Gormigrate) insertMigration(id string) error {
record := map[string]interface{}{g.options.IDColumnName: id}
return g.tx.Table(g.options.TableName).Create(record).Error
return g.tx.Exec("INSERT INTO " + g.options.TableName + " (" + g.options.IDColumnName + ") VALUES ('" + id + "')").Error

Choose a reason for hiding this comment

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

This can lead to SQL Injection and prevent id to have single quote inside. Am I missing something ?

Copy link
Author

Choose a reason for hiding this comment

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

It's not much of a PR, more like the draft of a possible fix to get some feedback

Copy link
Author

Choose a reason for hiding this comment

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

Just saw you made one at the same time, I'm closing this one

Choose a reason for hiding this comment

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

Oh I see ! No problem, I was just looking as I saw your PR just after I did one ahah.

@magellancl magellancl closed this Mar 18, 2024
@avakarev
Copy link
Contributor

@magellancl Thank you for your time and contribution.

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.

None yet

3 participants