Comment backport test and add missing drone test (#3127)

* comment backport test since the test reference many changes

* fix missing drone test on release/*

* remove test coverage on release/*

.drone.yml

@@ -86,6 +86,19 @@ pipeline:
       event: [ push, pull_request ]
       branch: [ master ]
 
+  test:
+    image: webhippie/golang:edge
+    pull: true
+    group: test
+    environment:
+      TAGS: bindata sqlite
+      GOPATH: /srv/app
+    commands:
+      - make test
+    when:
+      event: [ push, pull_request ]
+      branch: [ release/* ]
+
   test:
     image: webhippie/golang:edge
     pull: true

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## [1.3.1](https://github.com/go-gitea/gitea/releases/tag/v1.3.1) - 2017-12-08
+* BUGFIXES
+  * Sanitize logs for mirror sync (#3057, #3082) (#3078)
+  * Fix missing branch in release bug (#3108) (#3117)
+  * Fix repo indexer and submodule bug (#3107) (#3110)
+  * Fix legacy URL redirects (#3100) (#3106)
+  * Fix redis session failed (#3086) (#3089)
+  * Fix issue list branch link broken (#3061) (#3070)
+  * Fix missing password length check when change password (#3039) (#3071)
+
 ## [1.3.0](https://github.com/go-gitea/gitea/releases/tag/v1.3.0) - 2017-11-29
 * BREAKING
   * Make URL scheme unambiguous (#2408)

integrations/links_test.go

@@ -46,8 +46,10 @@ func TestRedirectsNoLogin(t *testing.T) {
 	prepareTestEnv(t)
 
 	var redirects = map[string]string{
-		"/user2/repo1/commits/master": "/user2/repo1/commits/branch/master",
-		"/user2/repo1/src/master":     "/user2/repo1/src/branch/master",
+		"/user2/repo1/commits/master":                "/user2/repo1/commits/branch/master",
+		"/user2/repo1/src/master":                    "/user2/repo1/src/branch/master",
+		"/user2/repo1/src/master/file.txt":           "/user2/repo1/src/branch/master/file.txt",
+		"/user2/repo1/src/master/directory/file.txt": "/user2/repo1/src/branch/master/directory/file.txt",
 	}
 	for link, redirectLink := range redirects {
 		req := NewRequest(t, "GET", link)

models/repo.go

@@ -605,9 +605,14 @@ func (repo *Repository) RepoPath() string {
 	return repo.repoPath(x)
 }
 
+// GitConfigPath returns the path to a repository's git config/ directory
+func GitConfigPath(repoPath string) string {
+	return filepath.Join(repoPath, "config")
+}
+
 // GitConfigPath returns the repository git config path
 func (repo *Repository) GitConfigPath() string {
-	return filepath.Join(repo.RepoPath(), "config")
+	return GitConfigPath(repo.RepoPath())
 }
 
 // RelLink returns the repository relative link

models/repo_indexer.go

@@ -100,10 +100,6 @@ func populateRepoIndexer() error {
 	}
 }
 
-type updateBatch struct {
-	updates []indexer.RepoIndexerUpdate
-}
-
 func updateRepoIndexer(repo *Repository) error {
 	changes, err := getRepoChanges(repo)
 	if err != nil {
@@ -163,6 +159,10 @@ func addUpdate(filename string, repo *Repository, batch *indexer.Batch) error {
 		return err
 	} else if stat.Size() > setting.Indexer.MaxIndexerFileSize {
 		return nil
+	} else if stat.IsDir() {
+		// file could actually be a directory, if it is the root of a submodule.
+		// We do not index submodule contents, so don't do anything.
+		return nil
 	}
 	fileContents, err := ioutil.ReadFile(filepath)
 	if err != nil {

models/repo_mirror.go

@@ -6,18 +6,18 @@ package models
 
 import (
 	"fmt"
-	"strings"
 	"time"
 
-	"github.com/Unknwon/com"
-	"github.com/go-xorm/xorm"
-	"gopkg.in/ini.v1"
-
 	"code.gitea.io/git"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/process"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/sync"
+	"code.gitea.io/gitea/modules/util"
+
+	"github.com/Unknwon/com"
+	"github.com/go-xorm/xorm"
+	"gopkg.in/ini.v1"
 )
 
 // MirrorQueue holds an UniqueQueue object of the mirror
@@ -76,41 +76,41 @@ func (m *Mirror) ScheduleNextUpdate() {
 	m.NextUpdate = time.Now().Add(m.Interval)
 }
 
+func remoteAddress(repoPath string) (string, error) {
+	cfg, err := ini.Load(GitConfigPath(repoPath))
+	if err != nil {
+		return "", err
+	}
+	return cfg.Section("remote \"origin\"").Key("url").Value(), nil
+}
+
 func (m *Mirror) readAddress() {
 	if len(m.address) > 0 {
 		return
 	}
-
-	cfg, err := ini.Load(m.Repo.GitConfigPath())
+	var err error
+	m.address, err = remoteAddress(m.Repo.RepoPath())
 	if err != nil {
-		log.Error(4, "Load: %v", err)
-		return
+		log.Error(4, "remoteAddress: %v", err)
 	}
-	m.address = cfg.Section("remote \"origin\"").Key("url").Value()
 }
 
-// HandleCloneUserCredentials replaces user credentials from HTTP/HTTPS URL
-// with placeholder <credentials>.
-// It will fail for any other forms of clone addresses.
-func HandleCloneUserCredentials(url string, mosaics bool) string {
-	i := strings.Index(url, "@")
-	if i == -1 {
-		return url
+// sanitizeOutput sanitizes output of a command, replacing occurrences of the
+// repository's remote address with a sanitized version.
+func sanitizeOutput(output, repoPath string) (string, error) {
+	remoteAddr, err := remoteAddress(repoPath)
+	if err != nil {
+		// if we're unable to load the remote address, then we're unable to
+		// sanitize.
+		return "", err
 	}
-	start := strings.Index(url, "://")
-	if start == -1 {
-		return url
-	}
-	if mosaics {
-		return url[:start+3] + "<credentials>" + url[i:]
-	}
-	return url[:start+3] + url[i+1:]
+	return util.SanitizeMessage(output, remoteAddr), nil
 }
 
 // Address returns mirror address from Git repository config without credentials.
 func (m *Mirror) Address() string {
 	m.readAddress()
-	return HandleCloneUserCredentials(m.address, false)
+	return util.SanitizeURLCredentials(m.address, false)
 }
 
 // FullAddress returns mirror address from Git repository config.
@@ -145,7 +145,14 @@ func (m *Mirror) runSync() bool {
 	if _, stderr, err := process.GetManager().ExecDir(
 		timeout, repoPath, fmt.Sprintf("Mirror.runSync: %s", repoPath),
 		"git", gitArgs...); err != nil {
-		desc := fmt.Sprintf("Failed to update mirror repository '%s': %s", repoPath, stderr)
+		// sanitize the output, since it may contain the remote address, which may
+		// contain a password
+		message, err := sanitizeOutput(stderr, repoPath)
+		if err != nil {
+			log.Error(4, "sanitizeOutput: %v", err)
+			return false
+		}
+		desc := fmt.Sprintf("Failed to update mirror repository '%s': %s", repoPath, message)
 		log.Error(4, desc)
 		if err = CreateRepositoryNotice(desc); err != nil {
 			log.Error(4, "CreateRepositoryNotice: %v", err)
@@ -170,7 +177,14 @@ func (m *Mirror) runSync() bool {
 		if _, stderr, err := process.GetManager().ExecDir(
 			timeout, wikiPath, fmt.Sprintf("Mirror.runSync: %s", wikiPath),
 			"git", "remote", "update", "--prune"); err != nil {
-			desc := fmt.Sprintf("Failed to update mirror wiki repository '%s': %s", wikiPath, stderr)
+			// sanitize the output, since it may contain the remote address, which may
+			// contain a password
+			message, err := sanitizeOutput(stderr, wikiPath)
+			if err != nil {
+				log.Error(4, "sanitizeOutput: %v", err)
+				return false
+			}
+			desc := fmt.Sprintf("Failed to update mirror wiki repository '%s': %s", wikiPath, message)
 			log.Error(4, desc)
 			if err = CreateRepositoryNotice(desc); err != nil {
 				log.Error(4, "CreateRepositoryNotice: %v", err)

modules/context/repo.go

@@ -618,7 +618,11 @@ func RepoRefByType(refType RepoRefType) macaron.Handler {
 
 			if refType == RepoRefLegacy {
 				// redirect from old URL scheme to new URL scheme
-				ctx.Redirect(path.Join(setting.AppSubURL, strings.TrimSuffix(ctx.Req.URL.String(), ctx.Params("*")), ctx.Repo.BranchNameSubURL()))
+				ctx.Redirect(path.Join(
+					setting.AppSubURL,
+					strings.TrimSuffix(ctx.Req.URL.String(), ctx.Params("*")),
+					ctx.Repo.BranchNameSubURL(),
+					ctx.Repo.TreePath))
 				return
 			}
 		}

modules/mailer/mailer.go

@@ -208,7 +208,7 @@ func (s *sendmailSender) Send(from string, to []string, msg io.WriterTo) error {
 	var closeError error
 	var waitError error
 
-	args := []string{"-F", from, "-i"}
+	args := []string{"-f", from, "-i"}
 	args = append(args, setting.MailService.SendmailArgs...)
 	args = append(args, to...)
 	log.Trace("Sending with: %s %v", setting.MailService.SendmailPath, args)

modules/setting/setting.go

@@ -1392,7 +1392,7 @@ func newSessionService() {
 	SessionConfig.Provider = Cfg.Section("session").Key("PROVIDER").In("memory",
 		[]string{"memory", "file", "redis", "mysql"})
 	SessionConfig.ProviderConfig = strings.Trim(Cfg.Section("session").Key("PROVIDER_CONFIG").MustString(path.Join(AppDataPath, "sessions")), "\" ")
-	if !filepath.IsAbs(SessionConfig.ProviderConfig) {
+	if SessionConfig.Provider == "file" && !filepath.IsAbs(SessionConfig.ProviderConfig) {
 		SessionConfig.ProviderConfig = path.Join(AppWorkPath, SessionConfig.ProviderConfig)
 	}
 	SessionConfig.CookieName = Cfg.Section("session").Key("COOKIE_NAME").MustString("i_like_gitea")

modules/test/context_tests.go

@@ -11,6 +11,7 @@ import (
 
 	"code.gitea.io/gitea/modules/context"
 
+	"github.com/go-macaron/session"
 	"github.com/stretchr/testify/assert"
 	macaron "gopkg.in/macaron.v1"
 )
@@ -33,6 +34,9 @@ func MockContext(t *testing.T) *context.Context {
 	macaronContext.Render = &mockRender{ResponseWriter: macaronContext.Resp}
 	return &context.Context{
 		Context: macaronContext,
+		Flash: &session.Flash{
+			Values: make(url.Values),
+		},
 	}
 }
 

modules/util/sanitize.go

@@ -0,0 +1,48 @@
+// Copyright 2017 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package util
+
+import (
+	"net/url"
+	"strings"
+)
+
+// urlSafeError wraps an error whose message may contain a sensitive URL
+type urlSafeError struct {
+	err            error
+	unsanitizedURL string
+}
+
+func (err urlSafeError) Error() string {
+	return SanitizeMessage(err.err.Error(), err.unsanitizedURL)
+}
+
+// URLSanitizedError returns the sanitized version an error whose message may
+// contain a sensitive URL
+func URLSanitizedError(err error, unsanitizedURL string) error {
+	return urlSafeError{err: err, unsanitizedURL: unsanitizedURL}
+}
+
+// SanitizeMessage sanitizes a message which may contains a sensitive URL
+func SanitizeMessage(message, unsanitizedURL string) string {
+	sanitizedURL := SanitizeURLCredentials(unsanitizedURL, true)
+	return strings.Replace(message, unsanitizedURL, sanitizedURL, -1)
+}
+
+// SanitizeURLCredentials sanitizes a url, either removing user credentials
+// or replacing them with a placeholder.
+func SanitizeURLCredentials(unsanitizedURL string, usePlaceholder bool) string {
+	u, err := url.Parse(unsanitizedURL)
+	if err != nil {
+		// don't log the error, since it might contain unsanitized URL.
+		return "(unparsable url)"
+	}
+	if u.User != nil && usePlaceholder {
+		u.User = url.User("<credentials>")
+	} else {
+		u.User = nil
+	}
+	return u.String()
+}

routers/api/v1/repo/repo.go

@@ -9,8 +9,6 @@ import (
 	"net/http"
 	"strings"
 
-	api "code.gitea.io/sdk/gitea"
-
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/auth"
 	"code.gitea.io/gitea/modules/context"
@@ -18,6 +16,7 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/routers/api/v1/convert"
+	api "code.gitea.io/sdk/gitea"
 )
 
 // Search repositories via options
@@ -322,12 +321,13 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) {
 		RemoteAddr:  remoteAddr,
 	})
 	if err != nil {
+		err = util.URLSanitizedError(err, remoteAddr)
 		if repo != nil {
 			if errDelete := models.DeleteRepository(ctx.User, ctxUser.ID, repo.ID); errDelete != nil {
 				log.Error(4, "DeleteRepository: %v", errDelete)
 			}
 		}
-		ctx.Error(500, "MigrateRepository", models.HandleCloneUserCredentials(err.Error(), true))
+		ctx.Error(500, "MigrateRepository", err)
 		return
 	}
 

routers/repo/release.go

@@ -191,6 +191,7 @@ func NewReleasePost(ctx *context.Context, form auth.NewReleaseForm) {
 
 		rel.Title = form.Title
 		rel.Note = form.Content
+		rel.Target = form.Target
 		rel.IsDraft = len(form.Draft) > 0
 		rel.IsPrerelease = form.Prerelease
 		rel.PublisherID = ctx.User.ID

routers/repo/repo.go

@@ -20,6 +20,7 @@ import (
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/util"
 )
 
 const (
@@ -232,6 +233,9 @@ func MigratePost(ctx *context.Context, form auth.MigrateRepoForm) {
 		return
 	}
 
+	// remoteAddr may contain credentials, so we sanitize it
+	err = util.URLSanitizedError(err, remoteAddr)
+
 	if repo != nil {
 		if errDelete := models.DeleteRepository(ctx.User, ctxUser.ID, repo.ID); errDelete != nil {
 			log.Error(4, "DeleteRepository: %v", errDelete)
@@ -241,11 +245,11 @@ func MigratePost(ctx *context.Context, form auth.MigrateRepoForm) {
 	if strings.Contains(err.Error(), "Authentication failed") ||
 		strings.Contains(err.Error(), "could not read Username") {
 		ctx.Data["Err_Auth"] = true
-		ctx.RenderWithErr(ctx.Tr("form.auth_failed", models.HandleCloneUserCredentials(err.Error(), true)), tplMigrate, &form)
+		ctx.RenderWithErr(ctx.Tr("form.auth_failed", err.Error()), tplMigrate, &form)
 		return
 	} else if strings.Contains(err.Error(), "fatal:") {
 		ctx.Data["Err_CloneAddr"] = true
-		ctx.RenderWithErr(ctx.Tr("repo.migrate.failed", models.HandleCloneUserCredentials(err.Error(), true)), tplMigrate, &form)
+		ctx.RenderWithErr(ctx.Tr("repo.migrate.failed", err.Error()), tplMigrate, &form)
 		return
 	}
 

routers/user/setting.go

@@ -223,7 +223,9 @@ func SettingsSecurityPost(ctx *context.Context, form auth.ChangePasswordForm) {
 		return
 	}
 
-	if ctx.User.IsPasswordSet() && !ctx.User.ValidatePassword(form.OldPassword) {
+	if len(form.Password) < setting.MinPasswordLength {
+		ctx.Flash.Error(ctx.Tr("auth.password_too_short", setting.MinPasswordLength))
+	} else if ctx.User.IsPasswordSet() && !ctx.User.ValidatePassword(form.OldPassword) {
 		ctx.Flash.Error(ctx.Tr("settings.password_incorrect"))
 	} else if form.Password != form.Retype {
 		ctx.Flash.Error(ctx.Tr("form.password_not_match"))

routers/user/setting_test.go

@@ -0,0 +1,56 @@
+// Copyright 2017 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package user
+
+/*func TestChangePassword(t *testing.T) {
+	oldPassword := "password"
+	setting.MinPasswordLength = 6
+
+	for _, req := range []struct {
+		OldPassword string
+		NewPassword string
+		Retype      string
+		Message     string
+	}{
+		{
+			OldPassword: oldPassword,
+			NewPassword: "123456",
+			Retype:      "123456",
+			Message:     "",
+		},
+		{
+			OldPassword: oldPassword,
+			NewPassword: "12345",
+			Retype:      "12345",
+			Message:     "auth.password_too_short",
+		},
+		{
+			OldPassword: "12334",
+			NewPassword: "123456",
+			Retype:      "123456",
+			Message:     "settings.password_incorrect",
+		},
+		{
+			OldPassword: oldPassword,
+			NewPassword: "123456",
+			Retype:      "12345",
+			Message:     "form.password_not_match",
+		},
+	} {
+		models.PrepareTestEnv(t)
+		ctx := test.MockContext(t, "user/settings/security")
+		test.LoadUser(t, ctx, 2)
+		test.LoadRepo(t, ctx, 1)
+
+		SettingsSecurityPost(ctx, auth.ChangePasswordForm{
+			OldPassword: req.OldPassword,
+			Password:    req.NewPassword,
+			Retype:      req.Retype,
+		})
+
+		assert.EqualValues(t, req.Message, ctx.Flash.ErrorMsg)
+		assert.EqualValues(t, http.StatusFound, ctx.Resp.Status())
+	}
+}*/

templates/repo/issue/list.tmpl

@@ -172,7 +172,7 @@
 					<a class="title has-emoji" href="{{$.Link}}/{{.Index}}">{{.Title}}</a>
 
 					{{if .Ref}}
-						<a class="ui label" href="{{$.RepoLink}}/src/commit/{{.Ref}}">{{.Ref}}</a>
+						<a class="ui label" href="{{$.RepoLink}}/src/branch/{{.Ref}}">{{.Ref}}</a>
 					{{end}}
 					{{range .Labels}}
 						<a class="ui label" href="{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&state={{$.State}}&labels={{.ID}}&milestone={{$.MilestoneID}}&assignee={{$.AssigneeID}}" style="color: {{.ForegroundColor}}; background-color: {{.Color}}">{{.Name}}</a>