From 51d86adb6d5451a2ac57e8c62b9beef524225a1d Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Tue, 25 Mar 2025 11:26:58 +0800
Subject: [PATCH] Fix some migration and repo name problems (#33986)
1. Ignore empty inputs in `UnmarshalHandleDoubleEncode`
2. Ignore non-existing `stateEvent.User` in gitlab migration
3. Enable `release` and `wiki` units when they are selected in migration
4. Sanitize repo name for migration and new repo
---
models/user/user_system.go | 10 ++++-----
modules/json/json.go | 6 +++++
modules/json/json_test.go | 18 +++++++++++++++
services/auth/oauth2_test.go | 2 +-
services/doctor/fix16961_test.go | 6 -----
services/migrations/gitlab.go | 9 ++++++--
services/repository/migrate.go | 14 ++++++++++++
tests/integration/mirror_pull_test.go | 30 +++++++++++++++++--------
web_src/js/features/repo-common.test.ts | 17 +++++++++++++-
web_src/js/features/repo-common.ts | 16 +++++++++++++
web_src/js/features/repo-migration.ts | 21 +++++++++++------
web_src/js/features/repo-new.ts | 5 +++++
12 files changed, 123 insertions(+), 31 deletions(-)
create mode 100644 modules/json/json_test.go
diff --git a/models/user/user_system.go b/models/user/user_system.go
index 6fbfd9e69e..e07274d291 100644
--- a/models/user/user_system.go
+++ b/models/user/user_system.go
@@ -10,8 +10,8 @@ import (
)
const (
- GhostUserID = -1
- GhostUserName = "Ghost"
+ GhostUserID int64 = -1
+ GhostUserName = "Ghost"
)
// NewGhostUser creates and returns a fake user for someone has deleted their account.
@@ -36,9 +36,9 @@ func (u *User) IsGhost() bool {
}
const (
- ActionsUserID = -2
- ActionsUserName = "gitea-actions"
- ActionsUserEmail = "teabot@gitea.io"
+ ActionsUserID int64 = -2
+ ActionsUserName = "gitea-actions"
+ ActionsUserEmail = "teabot@gitea.io"
)
func IsGiteaActionsUserName(name string) bool {
diff --git a/modules/json/json.go b/modules/json/json.go
index 34568c75c6..acd4118573 100644
--- a/modules/json/json.go
+++ b/modules/json/json.go
@@ -145,6 +145,12 @@ func Valid(data []byte) bool {
// UnmarshalHandleDoubleEncode - due to a bug in xorm (see https://gitea.com/xorm/xorm/pulls/1957) - it's
// possible that a Blob may be double encoded or gain an unwanted prefix of 0xff 0xfe.
func UnmarshalHandleDoubleEncode(bs []byte, v any) error {
+ if len(bs) == 0 {
+ // json.Unmarshal will report errors if input is empty (nil or zero-length)
+ // It seems that XORM ignores the nil but still passes zero-length string into this function
+ // To be consistent, we should treat all empty inputs as success
+ return nil
+ }
err := json.Unmarshal(bs, v)
if err != nil {
ok := true
diff --git a/modules/json/json_test.go b/modules/json/json_test.go
new file mode 100644
index 0000000000..ace7167913
--- /dev/null
+++ b/modules/json/json_test.go
@@ -0,0 +1,18 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package json
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+)
+
+func TestGiteaDBJSONUnmarshal(t *testing.T) {
+ var m map[any]any
+ err := UnmarshalHandleDoubleEncode(nil, &m)
+ assert.NoError(t, err)
+ err = UnmarshalHandleDoubleEncode([]byte(""), &m)
+ assert.NoError(t, err)
+}
diff --git a/services/auth/oauth2_test.go b/services/auth/oauth2_test.go
index 9edf18d58e..f003742a94 100644
--- a/services/auth/oauth2_test.go
+++ b/services/auth/oauth2_test.go
@@ -26,7 +26,7 @@ func TestUserIDFromToken(t *testing.T) {
o := OAuth2{}
uid := o.userIDFromToken(t.Context(), token, ds)
- assert.Equal(t, int64(user_model.ActionsUserID), uid)
+ assert.Equal(t, user_model.ActionsUserID, uid)
assert.Equal(t, true, ds["IsActionsToken"])
assert.Equal(t, ds["ActionsTaskID"], int64(RunningTaskID))
})
diff --git a/services/doctor/fix16961_test.go b/services/doctor/fix16961_test.go
index 498ed9c8d5..d5eded1117 100644
--- a/services/doctor/fix16961_test.go
+++ b/services/doctor/fix16961_test.go
@@ -18,12 +18,6 @@ func Test_fixUnitConfig_16961(t *testing.T) {
wantFixed bool
wantErr bool
}{
- {
- name: "empty",
- bs: "",
- wantFixed: true,
- wantErr: false,
- },
{
name: "normal: {}",
bs: "{}",
diff --git a/services/migrations/gitlab.go b/services/migrations/gitlab.go
index efc5b960cf..4bed8e2f6c 100644
--- a/services/migrations/gitlab.go
+++ b/services/migrations/gitlab.go
@@ -16,6 +16,7 @@ import (
"time"
issues_model "code.gitea.io/gitea/models/issues"
+ "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/container"
"code.gitea.io/gitea/modules/log"
base "code.gitea.io/gitea/modules/migration"
@@ -535,11 +536,15 @@ func (g *GitlabDownloader) GetComments(ctx context.Context, commentable base.Com
}
for _, stateEvent := range stateEvents {
+ posterUserID, posterUsername := user.GhostUserID, user.GhostUserName
+ if stateEvent.User != nil {
+ posterUserID, posterUsername = int64(stateEvent.User.ID), stateEvent.User.Username
+ }
comment := &base.Comment{
IssueIndex: commentable.GetLocalIndex(),
Index: int64(stateEvent.ID),
- PosterID: int64(stateEvent.User.ID),
- PosterName: stateEvent.User.Username,
+ PosterID: posterUserID,
+ PosterName: posterUsername,
Content: "",
Created: *stateEvent.CreatedAt,
}
diff --git a/services/repository/migrate.go b/services/repository/migrate.go
index 5b6feccb8d..9a5c6ffb0f 100644
--- a/services/repository/migrate.go
+++ b/services/repository/migrate.go
@@ -13,6 +13,7 @@ import (
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
repo_model "code.gitea.io/gitea/models/repo"
+ unit_model "code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/gitrepo"
@@ -246,6 +247,19 @@ func MigrateRepositoryGitData(ctx context.Context, u *user_model.User,
}
}
+ var enableRepoUnits []repo_model.RepoUnit
+ if opts.Releases && !unit_model.TypeReleases.UnitGlobalDisabled() {
+ enableRepoUnits = append(enableRepoUnits, repo_model.RepoUnit{RepoID: repo.ID, Type: unit_model.TypeReleases})
+ }
+ if opts.Wiki && !unit_model.TypeWiki.UnitGlobalDisabled() {
+ enableRepoUnits = append(enableRepoUnits, repo_model.RepoUnit{RepoID: repo.ID, Type: unit_model.TypeWiki})
+ }
+ if len(enableRepoUnits) > 0 {
+ err = UpdateRepositoryUnits(ctx, repo, enableRepoUnits, nil)
+ if err != nil {
+ return nil, err
+ }
+ }
return repo, committer.Commit()
}
diff --git a/tests/integration/mirror_pull_test.go b/tests/integration/mirror_pull_test.go
index cf6faa7704..7ab8f72b4a 100644
--- a/tests/integration/mirror_pull_test.go
+++ b/tests/integration/mirror_pull_test.go
@@ -4,10 +4,12 @@
package integration
import (
+ "slices"
"testing"
"code.gitea.io/gitea/models/db"
repo_model "code.gitea.io/gitea/models/repo"
+ "code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
@@ -19,11 +21,13 @@ import (
"code.gitea.io/gitea/tests"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
)
func TestMirrorPull(t *testing.T) {
defer tests.PrepareTestEnv(t)()
+ ctx := t.Context()
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
repoPath := repo_model.RepoPath(user.Name, repo.Name)
@@ -35,10 +39,10 @@ func TestMirrorPull(t *testing.T) {
Mirror: true,
CloneAddr: repoPath,
Wiki: true,
- Releases: false,
+ Releases: true,
}
- mirrorRepo, err := repo_service.CreateRepositoryDirectly(db.DefaultContext, user, user, repo_service.CreateRepoOptions{
+ mirrorRepo, err := repo_service.CreateRepositoryDirectly(ctx, user, user, repo_service.CreateRepoOptions{
Name: opts.RepoName,
Description: opts.Description,
IsPrivate: opts.Private,
@@ -48,11 +52,15 @@ func TestMirrorPull(t *testing.T) {
assert.NoError(t, err)
assert.True(t, mirrorRepo.IsMirror, "expected pull-mirror repo to be marked as a mirror immediately after its creation")
- ctx := t.Context()
-
- mirror, err := repo_service.MigrateRepositoryGitData(ctx, user, mirrorRepo, opts, nil)
+ mirrorRepo, err = repo_service.MigrateRepositoryGitData(ctx, user, mirrorRepo, opts, nil)
assert.NoError(t, err)
+ // these units should have been enabled
+ mirrorRepo.Units = nil
+ require.NoError(t, mirrorRepo.LoadUnits(ctx))
+ assert.True(t, slices.ContainsFunc(mirrorRepo.Units, func(u *repo_model.RepoUnit) bool { return u.Type == unit.TypeReleases }))
+ assert.True(t, slices.ContainsFunc(mirrorRepo.Units, func(u *repo_model.RepoUnit) bool { return u.Type == unit.TypeWiki }))
+
gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo)
assert.NoError(t, err)
defer gitRepo.Close()
@@ -60,10 +68,11 @@ func TestMirrorPull(t *testing.T) {
findOptions := repo_model.FindReleasesOptions{
IncludeDrafts: true,
IncludeTags: true,
- RepoID: mirror.ID,
+ RepoID: mirrorRepo.ID,
}
initCount, err := db.Count[repo_model.Release](db.DefaultContext, findOptions)
assert.NoError(t, err)
+ assert.Zero(t, initCount) // no sync yet, so even though there is a tag in source repo, the mirror's release table is still empty
assert.NoError(t, release_service.CreateRelease(gitRepo, &repo_model.Release{
RepoID: repo.ID,
@@ -79,12 +88,15 @@ func TestMirrorPull(t *testing.T) {
IsTag: true,
}, nil, ""))
- _, err = repo_model.GetMirrorByRepoID(ctx, mirror.ID)
+ _, err = repo_model.GetMirrorByRepoID(ctx, mirrorRepo.ID)
assert.NoError(t, err)
- ok := mirror_service.SyncPullMirror(ctx, mirror.ID)
+ ok := mirror_service.SyncPullMirror(ctx, mirrorRepo.ID)
assert.True(t, ok)
+ // actually there is a tag in the source repo, so after "sync", that tag will also come into the mirror
+ initCount++
+
count, err := db.Count[repo_model.Release](db.DefaultContext, findOptions)
assert.NoError(t, err)
assert.EqualValues(t, initCount+1, count)
@@ -93,7 +105,7 @@ func TestMirrorPull(t *testing.T) {
assert.NoError(t, err)
assert.NoError(t, release_service.DeleteReleaseByID(ctx, repo, release, user, true))
- ok = mirror_service.SyncPullMirror(ctx, mirror.ID)
+ ok = mirror_service.SyncPullMirror(ctx, mirrorRepo.ID)
assert.True(t, ok)
count, err = db.Count[repo_model.Release](db.DefaultContext, findOptions)
diff --git a/web_src/js/features/repo-common.test.ts b/web_src/js/features/repo-common.test.ts
index 009dfc86b1..33a29ecb2c 100644
--- a/web_src/js/features/repo-common.test.ts
+++ b/web_src/js/features/repo-common.test.ts
@@ -1,7 +1,22 @@
-import {substituteRepoOpenWithUrl} from './repo-common.ts';
+import {sanitizeRepoName, substituteRepoOpenWithUrl} from './repo-common.ts';
test('substituteRepoOpenWithUrl', () => {
// For example: "x-github-client://openRepo/https://github.com/go-gitea/gitea"
expect(substituteRepoOpenWithUrl('proto://a/{url}', 'https://gitea')).toEqual('proto://a/https://gitea');
expect(substituteRepoOpenWithUrl('proto://a?link={url}', 'https://gitea')).toEqual('proto://a?link=https%3A%2F%2Fgitea');
});
+
+test('sanitizeRepoName', () => {
+ expect(sanitizeRepoName(' a b ')).toEqual('a-b');
+ expect(sanitizeRepoName('a-b_c.git ')).toEqual('a-b_c');
+ expect(sanitizeRepoName('/x.git/')).toEqual('-x.git-');
+ expect(sanitizeRepoName('.profile')).toEqual('.profile');
+ expect(sanitizeRepoName('.profile.')).toEqual('.profile');
+ expect(sanitizeRepoName('.pro..file')).toEqual('.pro.file');
+
+ expect(sanitizeRepoName('foo.rss.atom.git.wiki')).toEqual('foo');
+
+ expect(sanitizeRepoName('.')).toEqual('');
+ expect(sanitizeRepoName('..')).toEqual('');
+ expect(sanitizeRepoName('-')).toEqual('');
+});
diff --git a/web_src/js/features/repo-common.ts b/web_src/js/features/repo-common.ts
index 4362a2c713..ebb6881c67 100644
--- a/web_src/js/features/repo-common.ts
+++ b/web_src/js/features/repo-common.ts
@@ -159,3 +159,19 @@ export async function updateIssuesMeta(url: string, action: string, issue_ids: s
console.error(error);
}
}
+
+export function sanitizeRepoName(name: string): string {
+ name = name.trim().replace(/[^-.\w]/g, '-');
+ for (let lastName = ''; lastName !== name;) {
+ lastName = name;
+ name = name.replace(/\.+$/g, '');
+ name = name.replace(/\.{2,}/g, '.');
+ for (const ext of ['.git', '.wiki', '.rss', '.atom']) {
+ if (name.endsWith(ext)) {
+ name = name.substring(0, name.length - ext.length);
+ }
+ }
+ }
+ if (['.', '..', '-'].includes(name)) name = '';
+ return name;
+}
diff --git a/web_src/js/features/repo-migration.ts b/web_src/js/features/repo-migration.ts
index fb9c822f98..4914e47267 100644
--- a/web_src/js/features/repo-migration.ts
+++ b/web_src/js/features/repo-migration.ts
@@ -1,4 +1,5 @@
import {hideElem, showElem, toggleElem} from '../utils/dom.ts';
+import {sanitizeRepoName} from './repo-common.ts';
const service = document.querySelector<HTMLInputElement>('#service_type');
const user = document.querySelector<HTMLInputElement>('#auth_username');
@@ -25,13 +26,19 @@ export function initRepoMigration() {
});
lfs?.addEventListener('change', setLFSSettingsVisibility);
- const cloneAddr = document.querySelector<HTMLInputElement>('#clone_addr');
- cloneAddr?.addEventListener('change', () => {
- const repoName = document.querySelector<HTMLInputElement>('#repo_name');
- if (cloneAddr.value && !repoName?.value) { // Only modify if repo_name input is blank
- repoName.value = /^(.*\/)?((.+?)(\.git)?)$/.exec(cloneAddr.value)[3];
- }
- });
+ const elCloneAddr = document.querySelector<HTMLInputElement>('#clone_addr');
+ const elRepoName = document.querySelector<HTMLInputElement>('#repo_name');
+ if (elCloneAddr && elRepoName) {
+ let repoNameChanged = false;
+ elRepoName.addEventListener('input', () => {repoNameChanged = true});
+ elCloneAddr.addEventListener('input', () => {
+ if (repoNameChanged) return;
+ let repoNameFromUrl = elCloneAddr.value.split(/[?#]/)[0];
+ repoNameFromUrl = /^(.*\/)?((.+?)\/?)$/.exec(repoNameFromUrl)[3];
+ repoNameFromUrl = repoNameFromUrl.split(/[?#]/)[0];
+ elRepoName.value = sanitizeRepoName(repoNameFromUrl);
+ });
+ }
}
function checkAuth() {
diff --git a/web_src/js/features/repo-new.ts b/web_src/js/features/repo-new.ts
index f2c5eba62c..5128942465 100644
--- a/web_src/js/features/repo-new.ts
+++ b/web_src/js/features/repo-new.ts
@@ -1,6 +1,7 @@
import {hideElem, showElem, toggleElem} from '../utils/dom.ts';
import {htmlEscape} from 'escape-goat';
import {fomanticQuery} from '../modules/fomantic/base.ts';
+import {sanitizeRepoName} from './repo-common.ts';
const {appSubUrl} = window.config;
@@ -74,6 +75,10 @@ export function initRepoNew() {
}
};
inputRepoName.addEventListener('input', updateUiRepoName);
+ inputRepoName.addEventListener('change', () => {
+ inputRepoName.value = sanitizeRepoName(inputRepoName.value);
+ updateUiRepoName();
+ });
updateUiRepoName();
initRepoNewTemplateSearch(form);