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

fix: AfterQuery using safer right trim while clearing from clause's j… #7153

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

bhowmik-abhijeet
Copy link
Contributor

Summary:
As a fix to #7025 , clearing of db.Statement.Clauses["FROM"] was in introduced in #7027

At https://github.com/go-gorm/gorm/blob/master/callbacks/query.go#L291 , it can cause a panic if db.Statement.Clauses["FROM"].Joins slice is being sliced using negative upper bound index
i.e. when len(db.Statement.Joins) > len(v.Joins). Please observe below the case reproduced in current source code

Steps to reproduce:

image

In the case above, wrong column name pets.names (instead of pets.name) is passed as query.
When Count() is called after Find() on the the same gorm.DB instance with a wrong query, it calls AfterQuery twice which in turn tries to trim the slice twice which now is empty after first trim. Hence instead of obtaining actual SQL error, it causes a panic due to index out of bound error.

Checklist:

  • Do only one thing
  • Non breaking API changes
  • Tested

What did this pull request do?:
This Pull Request:
Safely trims the db.Statement.Clauses["FROM"].Joins
Unit test for newly added functions for Safe right trimming of slice

User Case:
In one of my our projects, we are using gorm (just fantastic) as ORM support for Postgres. After upgrading gorm version from v1.9.16 -> v1.25.11, we identified panic in API due to broken migration which changes a column name causing the query to be wrong.

Of course this is a minor fix requirement but much needed to prevent API server error and handle SQL error gracefully.

@bhowmik-abhijeet
Copy link
Contributor Author

@liov Could you please kindly review and provide further suggestions? Thanks.

@jinzhu jinzhu merged commit 0daaf17 into go-gorm:master Aug 22, 2024
32 checks passed
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