-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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 wrong xorm Delete usage #27995
Fix wrong xorm Delete usage #27995
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: I didn't use db.WithTX
there because registerableSource.RegisterSource()
didn't have a context and it will use third-party function. Should the context be added?
CI failure is not related. Looks like the docker hub somehow not responding. |
Co-authored-by: delvh <dev.lh@web.de>
I was unable to create a backport for 1.20. @lng2020, please send one manually. 🍵
|
I was unable to create a backport for 1.21. @lng2020, please send one manually. 🍵
|
manually backport for #27995 The conflict is `ctx` and `db.Defaultctx`.
manually backport for #27995 The conflict is `ctx` and `db.Defaultctx`.
* giteaofficial/main: [skip ci] Updated licenses and gitignores Fix wrong xorm Delete usage (go-gitea#27995) Move some JS code from `fomantic.js` to standalone files (go-gitea#27994) Fix the wrong oauth2 name (go-gitea#27993) Render email addresses as such if followed by punctuation (go-gitea#27987) Show error toast when file size exceeds the limits (go-gitea#27985)
## Bug in Gitea I ran into this bug when I accidentally used the wrong redirect URL for the oauth2 provider when using mssql. But the oauth2 provider still got added. Most of the time, we use `Delete(&some{id: some.id})` or `In(condition).Delete(&some{})`, which specify the conditions. But the function uses `Delete(source)` when `source.Cfg` is a `TEXT` field and not empty. This will cause xorm `Delete` function not working in mssql. https://github.com/go-gitea/gitea/blob/61ff91f9603806df2505907614b9006bf721b9c8/models/auth/source.go#L234-L240 ## Reason Because the `TEXT` field can not be compared in mssql, xorm doesn't support it according to [this PR](https://gitea.com/xorm/xorm/pulls/2062) [related code](https://gitea.com/xorm/xorm/src/commit/b23798dc987af776bec867f4537ca129fd66328e/internal/statements/statement.go#L552-L558) in xorm ```go if statement.dialect.URI().DBType == schemas.MSSQL && (col.SQLType.Name == schemas.Text || col.SQLType.IsBlob() || col.SQLType.Name == schemas.TimeStampz) { if utils.IsValueZero(fieldValue) { continue } return nil, fmt.Errorf("column %s is a TEXT type with data %#v which cannot be as compare condition", col.Name, fieldValue.Interface()) } } ``` When using the `Delete` function in xorm, the non-empty fields will auto-set as conditions(perhaps some special fields are not?). If `TEXT` field is not empty, xorm will return an error. I only found this usage after searching, but maybe there is something I missing. --------- Co-authored-by: delvh <dev.lh@web.de>
Bug in Gitea
I ran into this bug when I accidentally used the wrong redirect URL for the oauth2 provider when using mssql. But the oauth2 provider still got added.
Most of the time, we use
Delete(&some{id: some.id})
orIn(condition).Delete(&some{})
, which specify the conditions. But the function usesDelete(source)
whensource.Cfg
is aTEXT
field and not empty. This will cause xormDelete
function not working in mssql.gitea/models/auth/source.go
Lines 234 to 240 in 61ff91f
Reason
Because the
TEXT
field can not be compared in mssql, xorm doesn't support it according to this PRrelated code in xorm
When using the
Delete
function in xorm, the non-empty fields will auto-set as conditions(perhaps some special fields are not?). IfTEXT
field is not empty, xorm will return an error. I only found this usage after searching, but maybe there is something I missing.