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

Markdown files cannot be edited #15932

Closed
a96219 opened this issue May 20, 2021 · 17 comments · Fixed by #16023
Closed

Markdown files cannot be edited #15932

a96219 opened this issue May 20, 2021 · 17 comments · Fixed by #16023
Labels

Comments

@a96219
Copy link

a96219 commented May 20, 2021

@a96219
Copy link
Author

a96219 commented May 20, 2021

image

@a96219
Copy link
Author

a96219 commented May 20, 2021

Click Edit to stay stuck in this interface

@silverwind
Copy link
Member

Check the browser console for any errors.

@a96219 a96219 closed this as completed May 20, 2021
@a96219 a96219 reopened this May 20, 2021
@a96219
Copy link
Author

a96219 commented May 20, 2021

检查浏览器控制台是否有任何错误。

Check the browser console for any errors.

I didn't see any errors
This project can reproduce the problem: https://try.gitea.io/FreeSaltedFish/test

@silverwind
Copy link
Member

silverwind commented May 20, 2021

Works fine for me when I edit one of the files I can access. Might be a browser cache problem of you had visited before, try clearing it or CTRL-F5 a few times.

@a96219
Copy link
Author

a96219 commented May 20, 2021

Works fine for me when I edit one of the files I can access. Might be a browser cache problem of you had visited before, try clearing it or CTRL-F5 a few times.

This is not true for all files, but only for some.
Can you edit the md document in my link above?

@a96219
Copy link
Author

a96219 commented May 20, 2021

Works fine for me when I edit one of the files I can access. Might be a browser cache problem of you had visited before, try clearing it or CTRL-F5 a few times.

When I delete parts of the file, I can open the edit page again

@silverwind
Copy link
Member

silverwind commented May 20, 2021

I forked your repo and indeed that seems to be some issue with the GET https://try.gitea.io/silverwind/testedit/_edit/master/test.md request hanging, seems like a backend bug.

@a96219
Copy link
Author

a96219 commented May 20, 2021

I forked your repo and indeed that seems to be some issue with the GET https://try.gitea.io/silverwind/testedit/_edit/master/test.md request hanging, seems like a backend bug.

I updated my repo:https://try.gitea.io/FreeSaltedFish/test

Test1.md is only one line more than test2.md.
but Test1.md cannot be edited,Test2.md can be edited.

Test3.md is only one more '\n' than test2.md
but Test3.md cannot be edited,Test2.md can be edited.

Is it caused by some special sequence of bytes?

@noerw
Copy link
Member

noerw commented May 21, 2021

This is a really weird one.
It seems it's not about the length of the file, but some other condition. I could also produce this with giteas CHANGELOG.md..
I suspect this is because we have Transfer-Encoding: chunked for HTTP, and somehow somewhere the chunk length is miscalculated, sending non-zero when the response is actually finished..?

edit: I bisected, 270aab4 is the bad commit (from PR #15667)
@zeripath any ideas?

@a96219
Copy link
Author

a96219 commented May 28, 2021

Will this bug be fixed in the next version?

@noerw
Copy link
Member

noerw commented May 29, 2021

@FreeSaltedFish The bug didn't exist in the last release, this only happens in the current development version. So yes, we try to not introduce new bugs in releases, but can't give any guarantee. Feel free to look into it yourself if you want to get it fixed asap.


@zeripath I studied your changes in your cat-file --batch PR for a while, but didn't find any obvious bug. Two potential error conditions come to mind;

  1. somewhere we read the wrong amount of bytes, or return the wrong size
  2. there is a racecondition for reading multiple files
    • on _edit page, there are two calls to blob.DataAsync(): one for the file itself, and one for .editorconfig. Could there be a racecondition?
    • (I don't understand where CatFileBatch() ensures results are returned in order)

Could you take a look on this?

For reference, here we call to read the file

dataRc, err := blob.DataAsync()
if err != nil {
ctx.NotFound("blob.Data", err)
return
}
defer dataRc.Close()
ctx.Data["FileSize"] = blob.Size()
ctx.Data["FileName"] = blob.Name()
buf := make([]byte, 1024)
n, _ := dataRc.Read(buf)
buf = buf[:n]
// Only some file types are editable online as text.
if !base.IsRepresentableAsText(buf) {
ctx.NotFound("base.IsRepresentableAsText", nil)
return
}
d, _ := ioutil.ReadAll(dataRc)
buf = append(buf, d...)

which is similar to the (working) codepath for viewing a file:

rd := charset.ToUTF8WithFallbackReader(io.MultiReader(bytes.NewReader(buf), dataRc))

@zeripath
Copy link
Contributor

zeripath commented May 29, 2021

There's not really any ordering as writes and reads shouldn't really be Asynchronous writes and reads. I guess I might have missed one?

I'd have to replicate and test. The trick might be to re-get the cat-file-batch triple before use as there's a simple check if anything is buffered.

@zeripath
Copy link
Contributor

Ah - I think there might actually be an issue here!

The Close!

@zeripath
Copy link
Contributor

hmm... still can't replicate this.

From ee9f0a3970d5e5a91f236b25672fa24a79d7ac27 Mon Sep 17 00:00:00 2001
From: Andrew Thornton <art27@cantab.net>
Date: Sat, 29 May 2021 19:41:29 +0100
Subject: [PATCH] Close the dataRC reader sooner

Fix #15932

Signed-off-by: Andrew Thornton <art27@cantab.net>
---
 routers/repo/editor.go | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/routers/repo/editor.go b/routers/repo/editor.go
index 2cc5c1e7f..3b74373c2 100644
--- a/routers/repo/editor.go
+++ b/routers/repo/editor.go
@@ -106,7 +106,13 @@ func editFile(ctx *context.Context, isNewFile bool) {
 			ctx.NotFound("blob.Data", err)
 			return
 		}
-		defer dataRc.Close()
+		closed := false
+
+		defer func() {
+			if !closed {
+				dataRc.Close()
+			}
+		}()
 
 		ctx.Data["FileSize"] = blob.Size()
 		ctx.Data["FileName"] = blob.Name()
@@ -122,6 +128,11 @@ func editFile(ctx *context.Context, isNewFile bool) {
 		}
 
 		d, _ := ioutil.ReadAll(dataRc)
+		if err := dataRc.Close(); err != nil {
+			log.Error("Error whilst closing blob data: %v", err)
+		}
+		closed = true
+
 		buf = append(buf, d...)
 		if content, err := charset.ToUTF8WithErr(buf); err != nil {
 			log.Error("ToUTF8WithErr: %v", err)
-- 
2.31.1

Could you try applying the above patch to see if that solves your issue? (if copying&pasting you may need to add a blank line at the end)

@noerw
Copy link
Member

noerw commented May 29, 2021

@zeripath yeah nice, that fixes it!

zeripath added a commit to zeripath/gitea that referenced this issue May 29, 2021
Fix go-gitea#15932

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath
Copy link
Contributor

Strange that I can't replicate. There must be a race but I don't quite understand how.

zeripath added a commit that referenced this issue May 30, 2021
Fix #15932

Signed-off-by: Andrew Thornton <art27@cantab.net>
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this issue Aug 10, 2021
Fix go-gitea#15932

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
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.

4 participants