Markdown files cannot be edited

[a96219]

• Gitea version (or commit ref): docker 1.15.0+dev-327-gc636ef8f1

• Git version:

• Operating system: centos 7

• Database (use [x]):

  • PostgreSQL

• Can you reproduce the bug at https://try.gitea.io:

  • try.gitea.io/FreeSaltedFish/test

[a96219]

[a96219]

Click Edit to stay stuck in this interface

[silverwind]

Check the browser console for any errors.

[a96219]

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

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]

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]

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]

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]

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]

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]

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]

Will this bug be fixed in the next version?

[noerw]

@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

gitea/routers/repo/editor.go

Lines 104 to 125 in fb6c689

	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:

gitea/routers/repo/view.go

Line 497 in fb6c689

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

[zeripath]

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]

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

The Close!

[zeripath]

hmm... still can't replicate this.

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

Fix #15932

Signed-off-by: Andrew Thornton <[email protected]>
---
 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]

@zeripath yeah nice, that fixes it!

[zeripath]

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