diff --git a/modules/charset/charset.go b/modules/charset/charset.go index 49b741d4ee4..8b902f3c341 100644 --- a/modules/charset/charset.go +++ b/modules/charset/charset.go @@ -52,7 +52,7 @@ func Lookup(label string) (e encoding.Encoding, name string) { type ConvertOpts struct { KeepBOM bool - ErrorReplacement []byte + ErrorReplacement []byte // leave it empty to remove the invalid bytes ErrorReturnOrigin bool } @@ -99,7 +99,12 @@ func ToUTF8DropErrors(content []byte) []byte { func ToUTF8(content []byte, opts ConvertOpts) []byte { charsetLabel, _ := DetectEncoding(content) if charsetLabel == "UTF-8" { - return maybeRemoveBOM(content, opts) + // even if the detected charset is UTF-8, the content may contain invalid UTF-8 sequences, so need to convert it to valid UTF-8 + content = maybeRemoveBOM(content, opts) + if opts.ErrorReturnOrigin { + return content + } + return bytes.ToValidUTF8(content, opts.ErrorReplacement) } encoding, _ := Lookup(charsetLabel) @@ -123,11 +128,7 @@ func ToUTF8(content []byte, opts ConvertOpts) []byte { if opts.ErrorReturnOrigin { return content } - if opts.ErrorReplacement == nil { - decoded = append(decoded, content[idx+n]) - } else { - decoded = append(decoded, opts.ErrorReplacement...) - } + decoded = append(decoded, opts.ErrorReplacement...) idx += n + 1 } return maybeRemoveBOM(decoded, opts) diff --git a/modules/charset/charset_test.go b/modules/charset/charset_test.go index ab1ef64e89f..0731b0c0e15 100644 --- a/modules/charset/charset_test.go +++ b/modules/charset/charset_test.go @@ -171,6 +171,8 @@ func TestToUTF8DropErrors(t *testing.T) { res = ToUTF8DropErrors([]byte{0x00, 0x00, 0x00, 0x00}) assert.Equal(t, []byte{0x00, 0x00, 0x00, 0x00}, res) + + assert.Equal(t, "caf ", string(ToUTF8DropErrors([]byte("caf\xe9")))) } func TestDetectEncoding(t *testing.T) { diff --git a/modules/json/json.go b/modules/json/json.go index 75c5f6ec97b..eea9a1e6e7b 100644 --- a/modules/json/json.go +++ b/modules/json/json.go @@ -23,7 +23,7 @@ type Decoder interface { // Interface represents an interface to handle json data type Interface interface { Marshal(v any) ([]byte, error) - MarshalWrite(w io.Writer, v any) error + MarshalWriteRelaxed(w io.Writer, v any) error Unmarshal(data []byte, v any) error NewEncoder(writer io.Writer) Encoder NewDecoder(reader io.Reader) Decoder @@ -37,9 +37,10 @@ func Marshal(v any) ([]byte, error) { return DefaultJSONHandler.Marshal(v) } -// MarshalWrite writes the JSON encoding of v to the given writer -func MarshalWrite(w io.Writer, v any) error { - return DefaultJSONHandler.MarshalWrite(w, v) +// MarshalWriteRelaxed writes the JSON encoding of v to the given writer +// It tolerates invalid UTF-8 in strings and replaces them with the Unicode replacement character U+FFFD. +func MarshalWriteRelaxed(w io.Writer, v any) error { + return DefaultJSONHandler.MarshalWriteRelaxed(w, v) } // Unmarshal decodes object from bytes diff --git a/modules/json/jsonv2.go b/modules/json/jsonv2.go index 08762f7e337..2d7f5d12682 100644 --- a/modules/json/jsonv2.go +++ b/modules/json/jsonv2.go @@ -13,6 +13,7 @@ import ( type JSONv2 struct { marshalOptions jsonv2.Options + marshalRelaxedOptions jsonv2.Options marshalKeepOptionalEmptyOptions jsonv2.Options unmarshalOptions jsonv2.Options unmarshalCaseInsensitiveOptions jsonv2.Options @@ -26,6 +27,8 @@ func init() { jsonv2.FormatNilMapAsNull(true), } jsonV2.marshalOptions = jsonv2.JoinOptions(commonMarshalOptions...) + // when handling user's data, we want to allow invalid UTF-8 to be replaced with the replacement character, rather than returning an error. + jsonV2.marshalRelaxedOptions = jsonv2.JoinOptions(append(commonMarshalOptions, jsontext.AllowInvalidUTF8(true))...) jsonV2.unmarshalOptions = jsonv2.DefaultOptionsV2() // By default, "json/v2" omitempty removes all `""` empty strings, no matter where it comes from. @@ -53,8 +56,8 @@ func (j *JSONv2) Marshal(v any) ([]byte, error) { return jsonv2.Marshal(v, j.marshalOptions) } -func (j *JSONv2) MarshalWrite(w io.Writer, v any) error { - return jsonv2.MarshalWrite(w, v, j.marshalOptions) +func (j *JSONv2) MarshalWriteRelaxed(w io.Writer, v any) error { + return jsonv2.MarshalWrite(w, v, j.marshalRelaxedOptions) } func (j *JSONv2) Unmarshal(data []byte, v any) error { diff --git a/modules/packages/composer/metadata.go b/modules/packages/composer/metadata.go index 2fec0b31457..27ad4222960 100644 --- a/modules/packages/composer/metadata.go +++ b/modules/packages/composer/metadata.go @@ -15,6 +15,7 @@ import ( "regexp" "strings" + "gitea.dev/modules/charset" "gitea.dev/modules/json" "gitea.dev/modules/util" "gitea.dev/modules/validation" @@ -261,6 +262,7 @@ func ParsePackage(r ReadSeekAt, optVersion ...string) (*PackageInfo, error) { cj.Readme = "README.md" } dataReadmeMd, _ := readPackageFile(pkgExt, r, cj.Readme, 10*1024) + dataReadmeMd = charset.ToUTF8DropErrors(dataReadmeMd) // FIXME: legacy problem, the "Readme" field is abused, it should always be the path to the readme file if len(dataReadmeMd) == 0 { diff --git a/modules/packages/composer/metadata_test.go b/modules/packages/composer/metadata_test.go index 48b9986405b..ca9abac3859 100644 --- a/modules/packages/composer/metadata_test.go +++ b/modules/packages/composer/metadata_test.go @@ -4,8 +4,6 @@ package composer import ( - "archive/tar" - "archive/zip" "bytes" "compress/gzip" "io" @@ -13,6 +11,7 @@ import ( "testing" "gitea.dev/modules/json" + "gitea.dev/modules/test" "github.com/dsnet/compress/bzip2" "github.com/stretchr/testify/assert" @@ -76,37 +75,8 @@ func TestCommentsUnmarshal(t *testing.T) { } func TestParsePackage(t *testing.T) { - createArchive := func(files map[string]string) []byte { - var buf bytes.Buffer - archive := zip.NewWriter(&buf) - for name, content := range files { - w, _ := archive.Create(name) - _, _ = w.Write([]byte(content)) - } - _ = archive.Close() - return buf.Bytes() - } - - createArchiveTar := func(comp func(io.Writer) io.WriteCloser, files map[string]string) []byte { - var buf bytes.Buffer - w := comp(&buf) - archive := tar.NewWriter(w) - for name, content := range files { - hdr := &tar.Header{ - Name: name, - Mode: 0o600, - Size: int64(len(content)), - } - _ = archive.WriteHeader(hdr) - _, _ = archive.Write([]byte(content)) - } - _ = w.Close() - _ = archive.Close() - return buf.Bytes() - } - t.Run("MissingComposerFile", func(t *testing.T) { - data := createArchive(map[string]string{"dummy.txt": ""}) + data := test.WriteZipArchive(map[string]string{"dummy.txt": ""}).Bytes() cp, err := ParsePackage(bytes.NewReader(data)) assert.Nil(t, cp) @@ -114,7 +84,7 @@ func TestParsePackage(t *testing.T) { }) t.Run("MissingComposerFileInRoot", func(t *testing.T) { - data := createArchive(map[string]string{"sub/sub/composer.json": ""}) + data := test.WriteZipArchive(map[string]string{"sub/sub/composer.json": ""}).Bytes() cp, err := ParsePackage(bytes.NewReader(data)) assert.Nil(t, cp) @@ -122,7 +92,7 @@ func TestParsePackage(t *testing.T) { }) t.Run("InvalidComposerFile", func(t *testing.T) { - data := createArchive(map[string]string{"composer.json": ""}) + data := test.WriteZipArchive(map[string]string{"composer.json": ""}).Bytes() cp, err := ParsePackage(bytes.NewReader(data)) assert.Nil(t, cp) @@ -130,7 +100,7 @@ func TestParsePackage(t *testing.T) { }) t.Run("InvalidPackageName", func(t *testing.T) { - data := createArchive(map[string]string{"composer.json": "{}"}) + data := test.WriteZipArchive(map[string]string{"composer.json": "{}"}).Bytes() cp, err := ParsePackage(bytes.NewReader(data)) assert.Nil(t, cp) @@ -138,7 +108,7 @@ func TestParsePackage(t *testing.T) { }) t.Run("InvalidPackageVersion", func(t *testing.T) { - data := createArchive(map[string]string{"composer.json": `{"name": "gitea/composer-package", "version": "1.a.3"}`}) + data := test.WriteZipArchive(map[string]string{"composer.json": `{"name": "gitea/composer-package", "version": "1.a.3"}`}).Bytes() cp, err := ParsePackage(bytes.NewReader(data)) assert.Nil(t, cp) @@ -146,7 +116,7 @@ func TestParsePackage(t *testing.T) { }) t.Run("InvalidReadmePath", func(t *testing.T) { - data := createArchive(map[string]string{"composer.json": `{"name": "gitea/composer-package", "readme": "sub/README.md"}`}) + data := test.WriteZipArchive(map[string]string{"composer.json": `{"name": "gitea/composer-package", "readme": "sub/README.md"}`}).Bytes() cp, err := ParsePackage(bytes.NewReader(data)) assert.NoError(t, err) @@ -177,22 +147,29 @@ func TestParsePackage(t *testing.T) { } t.Run("ValidZip", func(t *testing.T) { - data := createArchive(map[string]string{"composer.json": buildComposerContent(""), "README.md": readme}) + data := test.WriteZipArchive(map[string]string{"composer.json": buildComposerContent(""), "README.md": readme}).Bytes() assertValidPackage(t, data, "", "gitea-composer-package.zip") }) t.Run("ValidTarBz2", func(t *testing.T) { - data := createArchiveTar(func(w io.Writer) io.WriteCloser { + data := test.WriteTarCompression(func(w io.Writer) io.WriteCloser { bz2Writer, _ := bzip2.NewWriter(w, nil) return bz2Writer - }, map[string]string{"composer.json": buildComposerContent("1.0"), "README.md": readme}) + }, map[string]string{"composer.json": buildComposerContent("1.0"), "README.md": readme}).Bytes() assertValidPackage(t, data, "1.0", "gitea-composer-package.1.0.tar.bz2") }) t.Run("ValidTarGz", func(t *testing.T) { - data := createArchiveTar(func(w io.Writer) io.WriteCloser { + data := test.WriteTarCompression(func(w io.Writer) io.WriteCloser { return gzip.NewWriter(w) - }, map[string]string{"composer.json": buildComposerContent(""), "README.md": readme}) + }, map[string]string{"composer.json": buildComposerContent(""), "README.md": readme}).Bytes() assertValidPackage(t, data, "", "gitea-composer-package.tar.gz") }) + + t.Run("ReadmeEncoding", func(t *testing.T) { + data := test.WriteZipArchive(map[string]string{"composer.json": buildComposerContent(""), "README.md": "🌞🌞"[:6]}).Bytes() + cp, err := ParsePackage(bytes.NewReader(data)) + require.NoError(t, err) + assert.Equal(t, "🌞 ", cp.Metadata.Readme) + }) } diff --git a/services/context/base.go b/services/context/base.go index edcf0fc2eba..4885a6da1d7 100644 --- a/services/context/base.go +++ b/services/context/base.go @@ -114,7 +114,7 @@ func (b *Base) HTTPError(status int, contents ...string) { func (b *Base) JSON(status int, content any) { b.Resp.Header().Set("Content-Type", "application/json;charset=utf-8") b.Resp.WriteHeader(status) - if err := json.MarshalWrite(b.Resp, content); err != nil { + if err := json.MarshalWriteRelaxed(b.Resp, content); err != nil { log.Error("Render JSON failed: %v", err) } } diff --git a/services/convert/git_commit.go b/services/convert/git_commit.go index 19fefb337f4..04d10022b06 100644 --- a/services/convert/git_commit.go +++ b/services/convert/git_commit.go @@ -189,17 +189,17 @@ func ToCommit(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Rep // Retrieve files affected by the commit if opts.Files { - fileStatus, err := git.GetCommitFileStatus(ctx, repo, commit.ID.String()) + fileStatusSummary, err := git.GetCommitFileStatus(ctx, repo, commit.ID.String()) if err != nil { return nil, err } - affectedFileList := make([]*api.CommitAffectedFiles, 0, len(fileStatus.Added)+len(fileStatus.Removed)+len(fileStatus.Modified)) - for filestatus, files := range map[string][]string{"added": fileStatus.Added, "removed": fileStatus.Removed, "modified": fileStatus.Modified} { + affectedFileList := make([]*api.CommitAffectedFiles, 0, len(fileStatusSummary.Added)+len(fileStatusSummary.Removed)+len(fileStatusSummary.Modified)) + for fileStatus, files := range map[string][]string{"added": fileStatusSummary.Added, "removed": fileStatusSummary.Removed, "modified": fileStatusSummary.Modified} { for _, filename := range files { affectedFileList = append(affectedFileList, &api.CommitAffectedFiles{ Filename: filename, - Status: filestatus, + Status: fileStatus, }) } } diff --git a/services/convert/pull_review.go b/services/convert/pull_review.go index 96b7410bc11..8928ed078ab 100644 --- a/services/convert/pull_review.go +++ b/services/convert/pull_review.go @@ -6,10 +6,13 @@ package convert import ( "context" "strings" + "unicode/utf8" issues_model "gitea.dev/models/issues" user_model "gitea.dev/models/user" + "gitea.dev/modules/charset" api "gitea.dev/modules/structs" + "gitea.dev/modules/util" ) // ToPullReview convert a review to api format @@ -127,9 +130,13 @@ func ToPullReviewComment(ctx context.Context, comment *issues_model.Comment, doe } func patch2diff(patch string) string { - split := strings.Split(patch, "\n@@") - if len(split) == 2 { - return "@@" + split[1] + idx := strings.Index(patch, "\n@@") + if idx == -1 { + return "" } - return "" + diffContent := patch[idx+1:] + // Although there might be an edge case that deleted lines and added lines are in different encodings, + // it's not worth to make things more complicated at the moment, and most code in modern codebases should be written in UTF-8. + hunk := charset.ToUTF8(util.UnsafeStringToBytes(diffContent), charset.ConvertOpts{ErrorReplacement: []byte(string(utf8.RuneError))}) + return util.UnsafeBytesToString(hunk) } diff --git a/services/convert/pull_review_test.go b/services/convert/pull_review_test.go index d65821b42cf..3346253cf4e 100644 --- a/services/convert/pull_review_test.go +++ b/services/convert/pull_review_test.go @@ -49,3 +49,7 @@ func Test_ToPullReview(t *testing.T) { assert.Len(t, prList, 1) }) } + +func TestPatch2DiffInvalidUTF8(t *testing.T) { + assert.Equal(t, "@@ -1 +1 @@\n+caf�", patch2diff("diff --git a/a b/a\n@@ -1 +1 @@\n+caf\xe9")) +} diff --git a/services/websocket/events.go b/services/websocket/events.go index 563a368c39b..c7c3ec052eb 100644 --- a/services/websocket/events.go +++ b/services/websocket/events.go @@ -38,7 +38,7 @@ func MakeUserEventMessage(eventType string, eventData any) []byte { buf := &bytes.Buffer{} buf.WriteString(eventType) buf.WriteByte('\n') - err := json.MarshalWrite(buf, &UserEventMessage[any]{EventType: eventType, EventData: eventData}) + err := json.MarshalWriteRelaxed(buf, &UserEventMessage[any]{EventType: eventType, EventData: eventData}) payloadBytes := bytes.TrimSuffix(buf.Bytes(), []byte("\n")) // json v1 adds extra "\n" but we don't want it if err != nil { setting.PanicInDevOrTesting("websocket: marshal event: %v", err)