ERROR: invalid character '\n' in string literal

[samangh]

• Gitea version (or commit ref): 1.8.0 built with go1.12.4
• Git version: 2.21.0
• Operating system: Debian
• Database (use [x]):
  • MySQL (migrated from NoSQL)
• Can you reproduce the bug at https://try.gitea.io:
  • Not relevant

Description

It looks like gitea doesn't cope well with LF chacracters in a commit message. I regularly get the following in my gitea log:

2019/04/28 19:08:17 [...src/reflect/value.go:447 call()] [E] json.Unmarshal:
{"Len":1,"Commits":[{"Sha1":"0a4a263bbbf6577ee0f811d1ffb5bc97b9e53440","Message":"Add KVM notes
","AuthorEmail":"[email protected]","AuthorName":"Saman Ghannadzadeh","CommitterEmail":"[email protected]","CommitterName":"Saman Ghannadzadeh","Timestamp":"2019-04-08T22:09:30+01:00"}],"CompareURL":""}
ERROR: invalid character '\n' in string literal

Edit: Turns out this is because the gitea dump doesn't escape some special characters, see below.

[zeripath]

Hmm this is more interesting than that.

The issue appears to be when unmarshalling some JSON - I suspect somewhere we're writing JSON by hand instead of using marshal, I wouldn't be surprised if " weren't being properly escaped and you could inject other attributes.

I don't think this is exploitable hence my explicit mentioning of this but it's certainly not good.

The problem is that the log message is not very helpful - the error is coming from within internal go code. Is there any more information nearby to help identify the naughty code? Have you been able to reproduce on master?

[zeripath]

So the likely source of the error message is:

gitea/modules/templates/helper.go

Lines 426 to 433 in c8fc7fc

	// ActionContent2Commits converts action content to push commits
	func ActionContent2Commits(act Actioner) *models.PushCommits {
	push := models.NewPushCommits()
	if err := json.Unmarshal([]byte(act.GetContent()), push); err != nil {
	log.Error(4, "json.Unmarshal:\n%s\nERROR: %v", act.GetContent(), err)
	}
	return push
	}

in particular:

gitea/modules/templates/helper.go

Line 429 in c8fc7fc

if err := json.Unmarshal([]byte(act.GetContent()), push); err != nil {

The funny thing is why is act.GetContent() not unmarshalable, when AFAIK it should be marshalled...

[zeripath]

ActionContent2Commits is used on the feeds screen:

gitea/templates/user/dashboard/feeds.tmpl

Lines 62 to 74 in c8fc7fc

	{{if or (eq .GetOpType 5) (eq .GetOpType 18)}}
	<div class="content">
	<ul>
	{{ $push := ActionContent2Commits .}}
	{{ $repoLink := .GetRepoLink}}
	{{if $push.Commits}}
	{{range $push.Commits}}
	<li><img class="img-8" src="{{$push.AvatarLink .AuthorEmail}}"> <a class="commit-id" href="{{$repoLink}}/commit/{{.Sha1}}">{{ShortSha .Sha1}}</a> <span class="text truncate light grey has-emoji">{{.Message}}</span></li>
	{{end}}
	{{end}}
	{{if and (gt $push.Len 1) $push.CompareURL}}<li><a href="{{AppSubUrl}}/{{$push.CompareURL}}">{{$.i18n.Tr "action.compare_commits" $push.Len}} »</a></li>{{end}}
	</ul>
	</div>

OpType 5 is ActionCommitRepo and 18 is ActionMirrorSyncPush

So I should focus at these two types of action

[zeripath]

models/action.go:CommitRepoAction Seems to definitely Marshal and has been marshalling for 3 years...

gitea/models/action.go

Line 630 in c8fc7fc

data, err := json.Marshal(opts.Commits)

[zeripath]

models/action.go:MirrorSyncPushAction also seems to definitely Marshal

gitea/models/action.go

Line 840 in c8fc7fc

data, err := json.Marshal(opts.Commits)

hmm...

[zeripath]

OK I don't understand - there doesn't seem to be any way this can happen unless the Action table contents have somehow been changed. Have you migrated from a different database format? Do you have some unusual SQLServer options set?

[samangh]

Thanks for looking into this @zeripath. I did indeed migrate from NoSQL to MySQL, following this guide. Essentially I used gitea dump to dump the database and restored the dump into MySQL.

Sorry, I didn't realise it was relevant. I've edited the opening post to reflect this. Looking at the Action table, this seems to be relevant entry:

INSERT INTO `action` (`id`, `user_id`, `op_type`, `act_user_id`, `repo_id`, `comment_id`, `is_deleted`, `ref_name`, `is_private`, `content`, `created_unix`) VALUES
(824, 1, 5, 1, 43, 0, 0, 'master', 1, '{\"Len\":1,\"Commits\":[{\"Sha1\":\"0a4a263bbbf6577ee0f811d1ffb5bc97b9e53440\",\"Message\":\"Add KVM notes\n\",\"AuthorEmail\":\"[email protected]\",\"AuthorName\":\"Saman Ghannadzadeh\",\"CommitterEmail\":\"[email protected]\",\"CommitterName\":\"Saman Ghannadzadeh\",\"Timestamp\":\"2019-04-08T22:09:30+01:00\"}],\"CompareURL\":\"\"}', 1554757787)

Notice the extra "\n" in the commit message field.

I looked at my backup from gitea dump before I switched to MySQL, and it's also in the dump:

INSERT INTO `action` (`id`, `user_id`, `op_type`, `act_user_id`, `repo_id`, `comment_id`, `is_deleted`, `ref_name`, `is_private`, `content`, `created_unix`) VALUES (824, 1, 5, 1, 43, 0, false, 'master', true, '{"Len":1,"Commits":[{"Sha1":"0a4a263bbbf6577ee0f811d1ffb5bc97b9e53440","Message":"Add KVM notes\n","AuthorEmail":"[email protected]","AuthorName":"Saman Ghannadzadeh","CommitterEmail":"[email protected]","CommitterName":"Saman Ghannadzadeh","Timestamp":"2019-04-08T22:09:30+01:00"}],"CompareURL":""}', 1554757787);

So it looks the problem is how gittea dump dumps the database.

[zeripath]

Ok, so it looks like dump is the cause of this - it really needs a thorough check up and I suspect that it's thoroughly broken - (I've been avoiding it because I know I'll need to learn how xorm works properly and there are plenty of other bugs that have my attention elsewhere.)

To fix your immediate problem you need to change those explicit '\n' to escaped '\\n' in your database.

I think there are two options for how to proceed with this issue: either rename this issue, or close and reopen a new one explicitly mentioning dump as the cause.

[samangh]

Thanks, I've renamed the issue as you suggested. In the meanwhile, I'll fix the '\n's directly in the database.

[samangh]

I just realised that this still happens for actions that occured after the database tranfer!

{"Len":1,"Commits":[{"Sha1":"9bcefe20da7ef584d94319684142ad2a72a97b11","Message":"Checkin\n","AuthorEmail":"[email protected]","AuthorName":"GHANNADZADEH Saman","CommitterEmail":"[email protected],"CommitterName":"GHANNADZADEH Saman","Timestamp":"2019-05-10T08:01:23+01:00"}],"CompareURL":"attendance/compare/c481ac28261a28f0d020ecdd6e125ff5bef5a33e...9bcefe20da7ef584d94319684142ad2a72a97b11"}

[zeripath]

Hmm. So you're getting the unmarshalling error for these commits too?

Right, confirm for me what database you're using. Is it mysql or mssql? I can't repeat this on MSSQL or sqlite.

[zeripath]

If you're definitely getting unmarshalling errors for these commits I'm highly suspicious that mysql (or at least your mysql instance) is unescaping things either when it's being read or it is being inserted.

Do me a favour test some commits that have backslashes in them and perhaps other places e.g. repository description.

So that would be add a literal \t in to a commit or a string \t etc in to the repository description.

[samangh]

I'm using MySQL.

I created a test commit with some special characters, and they seem to be escaped correctly:

INSERT INTO `action` (`id`, `user_id`, `op_type`, `act_user_id`, `repo_id`, `comment_id`, `is_deleted`, `ref_name`, `is_private`, `content`, `created_unix`) VALUES
(894, 1, 5, 1, 1, 0, 0, 'master', 1, '{\"Len\":1,\"Commits\":[{\"Sha1\":\"66fd8c3d224c1f358f57d242374563ad55543761\",\"Message\":\"test \\\\n \\\\r \\\\t\\n\",\"AuthorEmail\":\"[email protected]\",\"AuthorName\":\"Saman Ghannadzadeh\",\"CommitterEmail\":\"[email protected]\",\"CommitterName\":\"Saman Ghannadzadeh\",\"Timestamp\":\"2019-05-11T13:31:40+01:00\"}],\"CompareURL\":\"saman/dotfiles/compare/5f81ccf3c05313658d4a9edd817526046e6a8cef...66fd8c3d224c1f358f57d242374563ad55543761\"}', 1557577915);

I tried a few other commits with special characters, they all seem to work now. Please ignore my previous message. I upgraded to v1.8.1 a couple of days ago, may be that fixed it?

I'll close this issue and will create a new one about git dump, as to not confuse things.

[zeripath]

Cool. Thanks for looking at this.