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

Inconsistent strconv.Atoi behavior for 32bit release, lead to error or data corruption. #20161

Closed
wxiaoguang opened this issue Jun 28, 2022 · 6 comments · Fixed by #20371
Closed
Labels
issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP type/bug
Milestone

Comments

@wxiaoguang
Copy link
Contributor

strconv.Atoi has different behavior:

$ GOARCH=386 go run test-int.go
2147483647 (0x80bee40,0x8c8e000)

$ GOARCH=amd64 go run test-int.go
4294967295 (0x0,0x0)

$ cat test-int.go
package main

import "strconv"

func main() {
	println(strconv.Atoi("4294967295"))
}

For example, in XORM:

image

A user has reported that Gitea 1.17 failed with error:

...
2022/06/28 11:47:32 routers/common/db.go:34:InitDBEngine() [I] Backing off for 3 seconds
2022/06/28 11:47:35 routers/common/db.go:27:InitDBEngine() [I] ORM engine initialization attempt #9/10...
2022/06/28 11:47:35 routers/common/db.go:33:InitDBEngine() [E] ORM engine initialization attempt #9/10 failed. Error: sync database struct error: strconv.Atoi: parsing "4294967295": value out of range
2022/06/28 11:47:35 routers/common/db.go:34:InitDBEngine() [I] Backing off for 3 seconds
2022/06/28 11:47:38 routers/common/db.go:27:InitDBEngine() [I] ORM engine initialization attempt #10/10...
2022/06/28 11:47:38 routers/init.go:68:mustInitCtx() [F] code.gitea.io/gitea/routers/common.InitDBEngine(ctx) failed: sync database struct error: strconv.Atoi: parsing "4294967295": value out of range
@wxiaoguang wxiaoguang added type/bug issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP issue/workaround it is or has a workaround labels Jun 28, 2022
@aha-tech
Copy link

I'm having the same issue using gitea-1.17-linux-arm-6 on a Raspberry Pi 4 Model B (32-bit ARMv7)

@nesc1
Copy link

nesc1 commented Jul 12, 2022

Also here same problem when executing:

2022/07/12 13:09:39 models/db/engine.go:126:SyncAllTables() [I] [SQL] SELECT `COLUMN_NAME`, `IS_NULLABLE`, `COLUMN_DEFAULT`, `COLUMN_TYPE`, `COLUMN_KEY`, `EXTRA`, `COLUMN_COMMENT`, `CHARACTER_MAXIMUM_LENGTH`, (INSTR(VERSION(), 'maria') > 0 && (SUBSTRING_INDEX(VERSION(), '.', 1) > 10 || (SUBSTRING_INDEX(VERSION(), '.', 1) = 10 && (SUBSTRING_INDEX(SUBSTRING(VERSION(), 4), '.', 1) > 2 || (SUBSTRING_INDEX(SUBSTRING(VERSION(), 4), '.', 1) = 2 && SUBSTRING_INDEX(SUBSTRING(VERSION(), 6), '-', 1) >= 7))))) AS NEEDS_QUOTE FROM `INFORMATION_SCHEMA`.`COLUMNS` WHERE `TABLE_SCHEMA` = ? AND `TABLE_NAME` = ? ORDER BY `COLUMNS`.ORDINAL_POSITION ASC [gitea pull_auto_merge] - 2.49032ms
2022/07/12 13:09:39 routers/common/db.go:33:InitDBEngine() [E] ORM engine initialization attempt #6/10 failed. Error: sync database struct error: strconv.Atoi: parsing "4294967295": value out of range
2022/07/12 13:09:39 routers/common/db.go:34:InitDBEngine() [I] Backing off for 3 seconds

it seems that CHARACTER_MAXIMUM_LENGTH with value 4294967295 creates the problem...

Now server is unusable because I can not return to previous version because I did not backup the database....

@zeripath
Copy link
Contributor

func Atoi(s string) (int, error) {

on 32bit systems int is most likely to be int32 rather than int64.

The problem is that xorm *schemas.Column uses signed int for column length and should either: a) use int64 or b) uint or even uint64 - however this is a breaking change in xorm.

@zeripath
Copy link
Contributor

@wxiaoguang wxiaoguang removed the issue/workaround it is or has a workaround label Jul 13, 2022
@6543 6543 added this to the 1.17.0 milestone Jul 14, 2022
6543 added a commit that referenced this issue Jul 14, 2022
…0372)

Backport #20371

Xorm 1.3.2-0.20220714055524 contains a fix for interpreting db column sizes. Prior to this fix xorm would assume that the size of a column was within the range of an `int`. This is correct on 64bit machines where `int` is typical equivalent to `int64` however, on 32bit machines `int` tends to be `int32`. 

Unfortunately the size of a LONGTEXT field is actually `max_uint32`, thus using `strconv.Atoi` on these fields will fail and thus #20161 occurs on 32 bit arm. Xorm 1.3.2-0.20220714055524 changes this field to use int64 instead.

Fix  #20161
zeripath pushed a commit that referenced this issue Jul 14, 2022
Xorm 1.3.2-0.20220714055524 contains a fix for interpreting db column sizes. Prior to this fix xorm would assume that the size of a column was within the range of an `int`. This is correct on 64bit machines where `int` is typical equivalent to `int64` however, on 32bit machines `int` tends to be `int32`. 

Unfortunately the size of a LONGTEXT field is actually `max_uint32`, thus using `strconv.Atoi` on these fields will fail and thus #20161 occurs on 32 bit arm. Xorm 1.3.2-0.20220714055524 changes this field to use int64 instead.

Fix  #20161
@wxiaoguang

This comment was marked as outdated.

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Jul 15, 2022

If you have met this bug, please download latest from https://dl.gitea.io/gitea/

vsysoev pushed a commit to IntegraSDL/gitea that referenced this issue Aug 10, 2022
)

Xorm 1.3.2-0.20220714055524 contains a fix for interpreting db column sizes. Prior to this fix xorm would assume that the size of a column was within the range of an `int`. This is correct on 64bit machines where `int` is typical equivalent to `int64` however, on 32bit machines `int` tends to be `int32`. 

Unfortunately the size of a LONGTEXT field is actually `max_uint32`, thus using `strconv.Atoi` on these fields will fail and thus go-gitea#20161 occurs on 32 bit arm. Xorm 1.3.2-0.20220714055524 changes this field to use int64 instead.

Fix  go-gitea#20161
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants