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

Editing CSV after 2048 chars results in "Can't render this file because it has a wrong number of fields" error #17378

Closed
richmahn opened this issue Oct 20, 2021 · 3 comments · Fixed by #17406
Labels

Comments

@richmahn
Copy link
Contributor

richmahn commented Oct 20, 2021

Gitea Version

35b918f

Git Version

v1.17.2

Operating System

MacOS ARM1

How are you running Gitea?

Gitea developer, so running a build of the main branch on my local system.

Database

MySQL

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Description

If you have a CSV file with more than 2048 characters (bytes), and then edit a line past 2048 characters (bytes), you get this error (line is based on whatever line the 2048th character falls on):

Can't render this file because it has a wrong number of fields in line 30.

Screenshot:

image

URL: https://try.gitea.io/richmahn/test/commit/ce02bb6107c8d7ba1b8a8d30335658a84e4c9c50

Yet the regular diff view shows no columns were changed, and both the parent commit and new commit render as CSV tables:

image

Actually, when you even create a CSV file with more than 2048 characters, the diff of the new file even gives you this error, because the code is only rendering/diffing the first 2048 bytes, and thus if the 2048th byte mark cuts off other fields in that row, that row, 30 in this case, is seen as having the wrong number of columns. See: https://try.gitea.io/richmahn/test/commit/44d026d24ff571be9de86a90015dae07e02dbc29 (both diff and CSV table)

Screenshots

image

image

@6543 6543 added the type/bug label Oct 20, 2021
@richmahn
Copy link
Contributor Author

richmahn commented Oct 20, 2021

Ok, found it has to do with this setting here to read 4096 bytes from a blob:

https://github.com/go-gitea/gitea/blob/35b918f/modules/git/blob_nogogit.go#L57

@zeripath
Copy link
Contributor

Err it shouldn't have...

There was another bug somewhere that might have resolved this though

@richmahn
Copy link
Contributor Author

Oops, didn't mean to close!

@richmahn richmahn reopened this Oct 21, 2021
6543 pushed a commit that referenced this issue Oct 24, 2021
closed #17378 

Both errors from #17378 were caused by  #15175.

Problem 1 (error with added file):
`ToUTF8WithFallbackReader` creates a `MultiReader` from a `byte[2048]` and the remaining reader. `CreateReaderAndGuessDelimiter` tries to read 10000 bytes from this reader but only gets 2048 because that's the first reader in the `MultiReader`. Then the `if size < 1e4` thinks the input is at EOF and just returns that.

Problem 2 (error with changed file):
The blob reader gets defer closed. That was fine because the old version reads the whole file into memory. Now with the streaming version the close needs to defer after the method.
KN4CK3R added a commit to KN4CK3R/gitea that referenced this issue Oct 25, 2021
closed go-gitea#17378 

Both errors from go-gitea#17378 were caused by  go-gitea#15175.

Problem 1 (error with added file):
`ToUTF8WithFallbackReader` creates a `MultiReader` from a `byte[2048]` and the remaining reader. `CreateReaderAndGuessDelimiter` tries to read 10000 bytes from this reader but only gets 2048 because that's the first reader in the `MultiReader`. Then the `if size < 1e4` thinks the input is at EOF and just returns that.

Problem 2 (error with changed file):
The blob reader gets defer closed. That was fine because the old version reads the whole file into memory. Now with the streaming version the close needs to defer after the method.
zeripath added a commit that referenced this issue Oct 25, 2021
Backport #17406.

Closes #17378 

Both errors from #17378 were caused by  #15175.

Problem 1 (error with added file):
`ToUTF8WithFallbackReader` creates a `MultiReader` from a `byte[2048]` and the remaining reader. `CreateReaderAndGuessDelimiter` tries to read 10000 bytes from this reader but only gets 2048 because that's the first reader in the `MultiReader`. Then the `if size < 1e4` thinks the input is at EOF and just returns that.

Problem 2 (error with changed file):
The blob reader gets defer closed. That was fine because the old version reads the whole file into memory. Now with the streaming version the close needs to defer after the method.

Co-authored-by: zeripath <[email protected]>
Chianina pushed a commit to Chianina/gitea that referenced this issue Mar 28, 2022
closed go-gitea#17378 

Both errors from go-gitea#17378 were caused by  go-gitea#15175.

Problem 1 (error with added file):
`ToUTF8WithFallbackReader` creates a `MultiReader` from a `byte[2048]` and the remaining reader. `CreateReaderAndGuessDelimiter` tries to read 10000 bytes from this reader but only gets 2048 because that's the first reader in the `MultiReader`. Then the `if size < 1e4` thinks the input is at EOF and just returns that.

Problem 2 (error with changed file):
The blob reader gets defer closed. That was fine because the old version reads the whole file into memory. Now with the streaming version the close needs to defer after the method.
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants