Use a more general (and faster) method to sanitize URLs with credentials (#19239)

Use a more general method to sanitize URLs with credentials: Simple and intuitive / Faster /  Remove all credentials in all URLs

4 files changed, 21 insertions, 34 deletions

diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go
index 45749aebfd..aa9fb4cccb 100644
--- a/services/mirror/mirror_pull.go
+++ b/services/mirror/mirror_pull.go
@@ -40,7 +40,7 @@ func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error
 
 	cmd := git.NewCommand(ctx, "remote", "add", remoteName, "--mirror=fetch", addr)
 	if strings.Contains(addr, "://") && strings.Contains(addr, "@") {
-		cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=fetch %s [repo_path: %s]", remoteName, util.NewStringURLSanitizer(addr, true).Replace(addr), repoPath))
+		cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=fetch %s [repo_path: %s]", remoteName, util.SanitizeCredentialURLs(addr), repoPath))
 	} else {
 		cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=fetch %s [repo_path: %s]", remoteName, addr, repoPath))
 	}
@@ -60,7 +60,7 @@ func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error
 
 		cmd = git.NewCommand(ctx, "remote", "add", remoteName, "--mirror=fetch", wikiRemotePath)
 		if strings.Contains(wikiRemotePath, "://") && strings.Contains(wikiRemotePath, "@") {
-			cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=fetch %s [repo_path: %s]", remoteName, util.NewStringURLSanitizer(wikiRemotePath, true).Replace(wikiRemotePath), wikiPath))
+			cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=fetch %s [repo_path: %s]", remoteName, util.SanitizeCredentialURLs(wikiRemotePath), wikiPath))
 		} else {
 			cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=fetch %s [repo_path: %s]", remoteName, wikiRemotePath, wikiPath))
 		}
@@ -160,7 +160,6 @@ func pruneBrokenReferences(ctx context.Context,
 	repoPath string,
 	timeout time.Duration,
 	stdoutBuilder, stderrBuilder *strings.Builder,
-	sanitizer *strings.Replacer,
 	isWiki bool,
 ) error {
 	wiki := ""
@@ -184,8 +183,8 @@ func pruneBrokenReferences(ctx context.Context,
 
 		// sanitize the output, since it may contain the remote address, which may
 		// contain a password
-		stderrMessage := sanitizer.Replace(stderr)
-		stdoutMessage := sanitizer.Replace(stdout)
+		stderrMessage := util.SanitizeCredentialURLs(stderr)
+		stdoutMessage := util.SanitizeCredentialURLs(stdout)
 
 		log.Error("Failed to prune mirror repository %s%-v references:\nStdout: %s\nStderr: %s\nErr: %v", wiki, m.Repo, stdoutMessage, stderrMessage, pruneErr)
 		desc := fmt.Sprintf("Failed to prune mirror repository %s'%s' references: %s", wiki, repoPath, stderrMessage)
@@ -229,11 +228,9 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo
 		stdout := stdoutBuilder.String()
 		stderr := stderrBuilder.String()
 
-		// sanitize the output, since it may contain the remote address, which may
-		// contain a password
-		sanitizer := util.NewURLSanitizer(remoteAddr, true)
-		stderrMessage := sanitizer.Replace(stderr)
-		stdoutMessage := sanitizer.Replace(stdout)
+		// sanitize the output, since it may contain the remote address, which may contain a password
+		stderrMessage := util.SanitizeCredentialURLs(stderr)
+		stdoutMessage := util.SanitizeCredentialURLs(stdout)
 
 		// Now check if the error is a resolve reference due to broken reference
 		if strings.Contains(stderr, "unable to resolve reference") && strings.Contains(stderr, "reference broken") {
@@ -241,7 +238,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo
 			err = nil
 
 			// Attempt prune
-			pruneErr := pruneBrokenReferences(ctx, m, repoPath, timeout, &stdoutBuilder, &stderrBuilder, sanitizer, false)
+			pruneErr := pruneBrokenReferences(ctx, m, repoPath, timeout, &stdoutBuilder, &stderrBuilder, false)
 			if pruneErr == nil {
 				// Successful prune - reattempt mirror
 				stderrBuilder.Reset()
@@ -259,8 +256,8 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo
 
 					// sanitize the output, since it may contain the remote address, which may
 					// contain a password
-					stderrMessage = sanitizer.Replace(stderr)
-					stdoutMessage = sanitizer.Replace(stdout)
+					stderrMessage = util.SanitizeCredentialURLs(stderr)
+					stdoutMessage = util.SanitizeCredentialURLs(stdout)
 				}
 			}
 		}
@@ -322,19 +319,9 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo
 			stdout := stdoutBuilder.String()
 			stderr := stderrBuilder.String()
 
-			// sanitize the output, since it may contain the remote address, which may
-			// contain a password
-
-			remoteAddr, remoteErr := git.GetRemoteAddress(ctx, wikiPath, m.GetRemoteName())
-			if remoteErr != nil {
-				log.Error("SyncMirrors [repo: %-v Wiki]: unable to get GetRemoteAddress Error %v", m.Repo, remoteErr)
-			}
-
-			// sanitize the output, since it may contain the remote address, which may
-			// contain a password
-			sanitizer := util.NewURLSanitizer(remoteAddr, true)
-			stderrMessage := sanitizer.Replace(stderr)
-			stdoutMessage := sanitizer.Replace(stdout)
+			// sanitize the output, since it may contain the remote address, which may contain a password
+			stderrMessage := util.SanitizeCredentialURLs(stderr)
+			stdoutMessage := util.SanitizeCredentialURLs(stdout)
 
 			// Now check if the error is a resolve reference due to broken reference
 			if strings.Contains(stderrMessage, "unable to resolve reference") && strings.Contains(stderrMessage, "reference broken") {
@@ -342,7 +329,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo
 				err = nil
 
 				// Attempt prune
-				pruneErr := pruneBrokenReferences(ctx, m, repoPath, timeout, &stdoutBuilder, &stderrBuilder, sanitizer, true)
+				pruneErr := pruneBrokenReferences(ctx, m, repoPath, timeout, &stdoutBuilder, &stderrBuilder, true)
 				if pruneErr == nil {
 					// Successful prune - reattempt mirror
 					stderrBuilder.Reset()
@@ -358,8 +345,8 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo
 						}); err != nil {
 						stdout := stdoutBuilder.String()
 						stderr := stderrBuilder.String()
-						stderrMessage = sanitizer.Replace(stderr)
-						stdoutMessage = sanitizer.Replace(stdout)
+						stderrMessage = util.SanitizeCredentialURLs(stderr)
+						stdoutMessage = util.SanitizeCredentialURLs(stdout)
 					}
 				}
 			}
diff --git a/services/mirror/mirror_push.go b/services/mirror/mirror_push.go
index 742338ef5f..e5c4b39895 100644
--- a/services/mirror/mirror_push.go
+++ b/services/mirror/mirror_push.go
@@ -31,7 +31,7 @@ func AddPushMirrorRemote(ctx context.Context, m *repo_model.PushMirror, addr str
 	addRemoteAndConfig := func(addr, path string) error {
 		cmd := git.NewCommand(ctx, "remote", "add", "--mirror=push", m.RemoteName, addr)
 		if strings.Contains(addr, "://") && strings.Contains(addr, "@") {
-			cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=push %s [repo_path: %s]", m.RemoteName, util.NewStringURLSanitizer(addr, true).Replace(addr), path))
+			cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=push %s [repo_path: %s]", m.RemoteName, util.SanitizeCredentialURLs(addr), path))
 		} else {
 			cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=push %s [repo_path: %s]", m.RemoteName, addr, path))
 		}
@@ -147,7 +147,7 @@ func runPushSync(ctx context.Context, m *repo_model.PushMirror) error {
 			endpoint := lfs.DetermineEndpoint(remoteAddr.String(), "")
 			lfsClient := lfs.NewClient(endpoint, nil)
 			if err := pushAllLFSObjects(ctx, gitRepo, lfsClient); err != nil {
-				return util.NewURLSanitizedError(err, remoteAddr, true)
+				return util.SanitizeErrorCredentialURLs(err)
 			}
 		}
 
@@ -161,7 +161,7 @@ func runPushSync(ctx context.Context, m *repo_model.PushMirror) error {
 		}); err != nil {
 			log.Error("Error pushing %s mirror[%d] remote %s: %v", path, m.ID, m.RemoteName, err)
 
-			return util.NewURLSanitizedError(err, remoteAddr, true)
+			return util.SanitizeErrorCredentialURLs(err)
 		}
 
 		return nil
diff --git a/services/task/migrate.go b/services/task/migrate.go
index d6ff514320..6f35134525 100644
--- a/services/task/migrate.go
+++ b/services/task/migrate.go
@@ -129,7 +129,7 @@ func runMigrateTask(t *models.Task) (err error) {
 	}
 
 	// remoteAddr may contain credentials, so we sanitize it
-	err = util.NewStringURLSanitizedError(err, opts.CloneAddr, true)
+	err = util.SanitizeErrorCredentialURLs(err)
 	if strings.Contains(err.Error(), "Authentication failed") ||
 		strings.Contains(err.Error(), "could not read Username") {
 		return fmt.Errorf("Authentication failed: %v", err.Error())
diff --git a/services/task/task.go b/services/task/task.go
index 3f823fc224..9deb0286c5 100644
--- a/services/task/task.go
+++ b/services/task/task.go
@@ -77,7 +77,7 @@ func CreateMigrateTask(doer, u *user_model.User, opts base.MigrateOptions) (*mod
 	if err != nil {
 		return nil, err
 	}
-	opts.CloneAddr = util.NewStringURLSanitizer(opts.CloneAddr, true).Replace(opts.CloneAddr)
+	opts.CloneAddr = util.SanitizeCredentialURLs(opts.CloneAddr)
 	opts.AuthPasswordEncrypted, err = secret.EncryptSecret(setting.SecretKey, opts.AuthPassword)
 	if err != nil {
 		return nil, err