Changelog v1.13.4 (#14917)

* Changelog v1.13.4

* nit

CHANGELOG.md

@@ -4,6 +4,19 @@ This changelog goes through all the changes that have been made in each release
 without substantial changes to our git log; to see the highlights of what has
 been added to each release, please refer to the [blog](https://blog.gitea.io).
 
+## [1.13.4](https://github.com/go-gitea/gitea/releases/tag/v1.13.4) - 2021-03-07
+
+* SECURITY
+  * Fix issue popups (#14898) (#14899)
+* BUGFIXES
+  * Fix race in LFS ContentStore.Put(...) (#14895) (#14913)
+  * Fix a couple of issues with a feeds (#14897) (#14903)
+  * When transfering repository and database transaction failed, rollback the renames (#14864) (#14902)
+  * Fix race in local storage (#14888) (#14901)
+  * Fix 500 on pull view page if user is not loged in (#14885) (#14886)
+* DOCS
+  * Fix how lfs data path is set (#14855) (#14884)
+
 ## [1.13.3](https://github.com/go-gitea/gitea/releases/tag/v1.13.3) - 2021-03-04
 
 * BREAKING

docs/content/doc/advanced/config-cheat-sheet.en-us.md

@@ -276,7 +276,7 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`.
 - `LANDING_PAGE`: **home**: Landing page for unauthenticated users \[home, explore, organizations, login\].
 
 - `LFS_START_SERVER`: **false**: Enables git-lfs support.
-- `LFS_CONTENT_PATH`: **%(APP_DATA_PATH)/lfs**:  Default LFS content path. (if it is on local storage.)
+- `LFS_CONTENT_PATH`: **%(APP_DATA_PATH)/lfs**:  DEPRECATED: Default LFS content path. (if it is on local storage.)
 - `LFS_JWT_SECRET`: **\<empty\>**: LFS authentication secret, change this a unique string.
 - `LFS_HTTP_AUTH_EXPIRY`: **20m**: LFS authentication validity period in time.Duration, pushes taking longer than this may fail.
 - `LFS_MAX_FILE_SIZE`: **0**: Maximum allowed LFS file size in bytes (Set to 0 for no limit).
@@ -828,7 +828,7 @@ is `data/lfs` and the default of `MINIO_BASE_PATH` is `lfs/`.
 
 - `STORAGE_TYPE`: **local**: Storage type for lfs, `local` for local disk or `minio` for s3 compatible object storage service or other name defined with `[storage.xxx]`
 - `SERVE_DIRECT`: **false**: Allows the storage driver to redirect to authenticated URLs to serve files directly. Currently, only Minio/S3 is supported via signed URLs, local does nothing.
-- `CONTENT_PATH`: **./data/lfs**: Where to store LFS files, only available when `STORAGE_TYPE` is `local`.
+- `PATH`: **./data/lfs**: Where to store LFS files, only available when `STORAGE_TYPE` is `local`. If not set it fall back to deprecated LFS_CONTENT_PATH value in [server] section.
 - `MINIO_ENDPOINT`: **localhost:9000**: Minio endpoint to connect only available when `STORAGE_TYPE` is `minio`
 - `MINIO_ACCESS_KEY_ID`: Minio accessKeyID to connect only available when `STORAGE_TYPE` is `minio`
 - `MINIO_SECRET_ACCESS_KEY`: Minio secretAccessKey to connect only available when `STORAGE_TYPE is` `minio`

docs/content/doc/advanced/config-cheat-sheet.zh-cn.md

@@ -73,6 +73,7 @@ menu:
 
 - `LFS_START_SERVER`: 是否启用 git-lfs 支持. 可以为 `true` 或 `false`， 默认是 `false`。
 - `LFS_JWT_SECRET`: LFS 认证密钥，改成自己的。
+- `LFS_CONTENT_PATH`: **已废弃**, 存放 lfs 命令上传的文件的地方，默认是 `data/lfs`。
 
 ## Database (`database`)
 
@@ -323,7 +324,7 @@ LFS 的存储配置。 如果 `STORAGE_TYPE` 为空，则此配置将从 `[stora
 
 - `STORAGE_TYPE`: **local**: LFS 的存储类型，`local` 将存储到磁盘，`minio` 将存储到 s3 兼容的对象服务。
 - `SERVE_DIRECT`: **false**: 允许直接重定向到存储系统。当前，仅 Minio/S3 是支持的。
-- `CONTENT_PATH`: 存放 lfs 命令上传的文件的地方，默认是 `data/lfs`。
+- `PATH`: 存放 lfs 命令上传的文件的地方，默认是 `data/lfs`。
 - `MINIO_ENDPOINT`: **localhost:9000**: Minio 地址，仅当 `LFS_STORAGE_TYPE` 为 `minio` 时有效。
 - `MINIO_ACCESS_KEY_ID`: Minio accessKeyID，仅当 `LFS_STORAGE_TYPE` 为 `minio` 时有效。
 - `MINIO_SECRET_ACCESS_KEY`: Minio secretAccessKey，仅当 `LFS_STORAGE_TYPE` 为 `minio` 时有效。

models/repo.go

@@ -1290,11 +1290,44 @@ func IncrementRepoForkNum(ctx DBContext, repoID int64) error {
 }
 
 // TransferOwnership transfers all corresponding setting from old user to new one.
-func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error {
+func TransferOwnership(doer *User, newOwnerName string, repo *Repository) (err error) {
+	repoRenamed := false
+	wikiRenamed := false
+	oldOwnerName := doer.Name
+
+	defer func() {
+		if !repoRenamed && !wikiRenamed {
+			return
+		}
+
+		recoverErr := recover()
+		if err == nil && recoverErr == nil {
+			return
+		}
+
+		if repoRenamed {
+			if err := os.Rename(RepoPath(newOwnerName, repo.Name), RepoPath(oldOwnerName, repo.Name)); err != nil {
+				log.Critical("Unable to move repository %s/%s directory from %s back to correct place %s: %v", oldOwnerName, repo.Name, RepoPath(newOwnerName, repo.Name), RepoPath(oldOwnerName, repo.Name), err)
+			}
+		}
+
+		if wikiRenamed {
+			if err := os.Rename(WikiPath(newOwnerName, repo.Name), WikiPath(oldOwnerName, repo.Name)); err != nil {
+				log.Critical("Unable to move wiki for repository %s/%s directory from %s back to correct place %s: %v", oldOwnerName, repo.Name, WikiPath(newOwnerName, repo.Name), WikiPath(oldOwnerName, repo.Name), err)
+			}
+		}
+
+		if recoverErr != nil {
+			log.Error("Panic within TransferOwnership: %v\n%s", recoverErr, log.Stack(2))
+			panic(recoverErr)
+		}
+	}()
+
 	newOwner, err := GetUserByName(newOwnerName)
 	if err != nil {
 		return fmt.Errorf("get new owner '%s': %v", newOwnerName, err)
 	}
+	newOwnerName = newOwner.Name // ensure capitalisation matches
 
 	// Check if new owner has repository with same name.
 	has, err := IsRepositoryExist(newOwner, repo.Name)
@@ -1311,6 +1344,7 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error
 	}
 
 	oldOwner := repo.Owner
+	oldOwnerName = oldOwner.Name
 
 	// Note: we have to set value here to make sure recalculate accesses is based on
 	// new owner.
@@ -1370,9 +1404,9 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error
 	}
 
 	// Update repository count.
-	if _, err = sess.Exec("UPDATE `user` SET num_repos=num_repos+1 WHERE id=?", newOwner.ID); err != nil {
+	if _, err := sess.Exec("UPDATE `user` SET num_repos=num_repos+1 WHERE id=?", newOwner.ID); err != nil {
 		return fmt.Errorf("increase new owner repository count: %v", err)
-	} else if _, err = sess.Exec("UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", oldOwner.ID); err != nil {
+	} else if _, err := sess.Exec("UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", oldOwner.ID); err != nil {
 		return fmt.Errorf("decrease old owner repository count: %v", err)
 	}
 
@@ -1382,7 +1416,7 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error
 
 	// Remove watch for organization.
 	if oldOwner.IsOrganization() {
-		if err = watchRepo(sess, oldOwner.ID, repo.ID, false); err != nil {
+		if err := watchRepo(sess, oldOwner.ID, repo.ID, false); err != nil {
 			return fmt.Errorf("watchRepo [false]: %v", err)
 		}
 	}
@@ -1394,16 +1428,18 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error
 		return fmt.Errorf("Failed to create dir %s: %v", dir, err)
 	}
 
-	if err = os.Rename(RepoPath(oldOwner.Name, repo.Name), RepoPath(newOwner.Name, repo.Name)); err != nil {
+	if err := os.Rename(RepoPath(oldOwner.Name, repo.Name), RepoPath(newOwner.Name, repo.Name)); err != nil {
 		return fmt.Errorf("rename repository directory: %v", err)
 	}
+	repoRenamed = true
 
 	// Rename remote wiki repository to new path and delete local copy.
 	wikiPath := WikiPath(oldOwner.Name, repo.Name)
 	if com.IsExist(wikiPath) {
-		if err = os.Rename(wikiPath, WikiPath(newOwner.Name, repo.Name)); err != nil {
+		if err := os.Rename(wikiPath, WikiPath(newOwner.Name, repo.Name)); err != nil {
 			return fmt.Errorf("rename repository wiki: %v", err)
 		}
+		wikiRenamed = true
 	}
 
 	// If there was previously a redirect at this location, remove it.

modules/lfs/content_store.go

@@ -9,6 +9,7 @@ import (
 	"encoding/hex"
 	"errors"
 	"fmt"
+	"hash"
 	"io"
 	"os"
 
@@ -66,15 +67,20 @@ func (s *ContentStore) Get(meta *models.LFSMetaObject, fromByte int64) (io.ReadC
 
 // Put takes a Meta object and an io.Reader and writes the content to the store.
 func (s *ContentStore) Put(meta *models.LFSMetaObject, r io.Reader) error {
-	hash := sha256.New()
-	rd := io.TeeReader(r, hash)
 	p := meta.RelativePath()
-	written, err := s.Save(p, rd)
+
+	// Wrap the provided reader with an inline hashing and size checker
+	wrappedRd := newHashingReader(meta.Size, meta.Oid, r)
+
+	// now pass the wrapped reader to Save - if there is a size mismatch or hash mismatch then
+	// the errors returned by the newHashingReader should percolate up to here
+	written, err := s.Save(p, wrappedRd)
 	if err != nil {
 		log.Error("Whilst putting LFS OID[%s]: Failed to copy to tmpPath: %s Error: %v", meta.Oid, p, err)
 		return err
 	}
 
+	// This shouldn't happen but it is sensible to test
 	if written != meta.Size {
 		if err := s.Delete(p); err != nil {
 			log.Error("Cleaning the LFS OID[%s] failed: %v", meta.Oid, err)
@@ -82,14 +88,6 @@ func (s *ContentStore) Put(meta *models.LFSMetaObject, r io.Reader) error {
 		return errSizeMismatch
 	}
 
-	shaStr := hex.EncodeToString(hash.Sum(nil))
-	if shaStr != meta.Oid {
-		if err := s.Delete(p); err != nil {
-			log.Error("Cleaning the LFS OID[%s] failed: %v", meta.Oid, err)
-		}
-		return errHashMismatch
-	}
-
 	return nil
 }
 
@@ -118,3 +116,45 @@ func (s *ContentStore) Verify(meta *models.LFSMetaObject) (bool, error) {
 
 	return true, nil
 }
+
+type hashingReader struct {
+	internal     io.Reader
+	currentSize  int64
+	expectedSize int64
+	hash         hash.Hash
+	expectedHash string
+}
+
+func (r *hashingReader) Read(b []byte) (int, error) {
+	n, err := r.internal.Read(b)
+
+	if n > 0 {
+		r.currentSize += int64(n)
+		wn, werr := r.hash.Write(b[:n])
+		if wn != n || werr != nil {
+			return n, werr
+		}
+	}
+
+	if err != nil && err == io.EOF {
+		if r.currentSize != r.expectedSize {
+			return n, errSizeMismatch
+		}
+
+		shaStr := hex.EncodeToString(r.hash.Sum(nil))
+		if shaStr != r.expectedHash {
+			return n, errHashMismatch
+		}
+	}
+
+	return n, err
+}
+
+func newHashingReader(expectedSize int64, expectedHash string, reader io.Reader) *hashingReader {
+	return &hashingReader{
+		internal:     reader,
+		expectedSize: expectedSize,
+		expectedHash: expectedHash,
+		hash:         sha256.New(),
+	}
+}

modules/storage/local.go

@@ -7,6 +7,7 @@ package storage
 import (
 	"context"
 	"io"
+	"io/ioutil"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -24,13 +25,15 @@ const LocalStorageType Type = "local"
 
 // LocalStorageConfig represents the configuration for a local storage
 type LocalStorageConfig struct {
-	Path string `ini:"PATH"`
+	Path          string `ini:"PATH"`
+	TemporaryPath string `ini:"TEMPORARY_PATH"`
 }
 
 // LocalStorage represents a local files storage
 type LocalStorage struct {
-	ctx context.Context
+	ctx    context.Context
-	dir string
+	dir    string
+	tmpdir string
 }
 
 // NewLocalStorage returns a local files
@@ -45,9 +48,14 @@ func NewLocalStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error
 		return nil, err
 	}
 
+	if config.TemporaryPath == "" {
+		config.TemporaryPath = config.Path + "/tmp"
+	}
+
 	return &LocalStorage{
-		ctx: ctx,
+		ctx:    ctx,
-		dir: config.Path,
+		dir:    config.Path,
+		tmpdir: config.TemporaryPath,
 	}, nil
 }
 
@@ -63,17 +71,37 @@ func (l *LocalStorage) Save(path string, r io.Reader) (int64, error) {
 		return 0, err
 	}
 
-	// always override
+	// Create a temporary file to save to
-	if err := util.Remove(p); err != nil {
+	if err := os.MkdirAll(l.tmpdir, os.ModePerm); err != nil {
 		return 0, err
 	}
-
+	tmp, err := ioutil.TempFile(l.tmpdir, "upload-*")
-	f, err := os.Create(p)
 	if err != nil {
 		return 0, err
 	}
-	defer f.Close()
+	tmpRemoved := false
-	return io.Copy(f, r)
+	defer func() {
+		if !tmpRemoved {
+			_ = util.Remove(tmp.Name())
+		}
+	}()
+
+	n, err := io.Copy(tmp, r)
+	if err != nil {
+		return 0, err
+	}
+
+	if err := tmp.Close(); err != nil {
+		return 0, err
+	}
+
+	if err := os.Rename(tmp.Name(), p); err != nil {
+		return 0, err
+	}
+
+	tmpRemoved = true
+
+	return n, nil
 }
 
 // Stat returns the info of the file

modules/templates/helper.go

@@ -689,6 +689,11 @@ func ActionIcon(opType models.ActionType) string {
 // ActionContent2Commits converts action content to push commits
 func ActionContent2Commits(act Actioner) *repository.PushCommits {
 	push := repository.NewPushCommits()
+
+	if act == nil || act.GetContent() == "" {
+		return push
+	}
+
 	if err := json.Unmarshal([]byte(act.GetContent()), push); err != nil {
 		log.Error("json.Unmarshal:\n%s\nERROR: %v", act.GetContent(), err)
 	}

services/pull/update.go

@@ -48,6 +48,9 @@ func Update(pull *models.PullRequest, doer *models.User, message string) error {
 
 // IsUserAllowedToUpdate check if user is allowed to update PR with given permissions and branch protections
 func IsUserAllowedToUpdate(pull *models.PullRequest, user *models.User) (bool, error) {
+	if user == nil {
+		return false, nil
+	}
 	headRepoPerm, err := models.GetUserRepoPermission(pull.HeadRepo, user)
 	if err != nil {
 		return false, err

templates/user/dashboard/feeds.tmpl

@@ -96,7 +96,8 @@
 						<span class="text truncate issue title">{{index .GetIssueInfos 1 | RenderEmoji}}</span>
 					{{else if or (eq .GetOpType 10) (eq .GetOpType 21) (eq .GetOpType 22) (eq .GetOpType 23)}}
 						<a href="{{.GetCommentLink}}" class="text truncate issue title">{{.GetIssueTitle | RenderEmoji}}</a>
-						<p class="text light grey">{{index .GetIssueInfos 1 | RenderEmoji}}</p>
+						{{$comment := index .GetIssueInfos 1}}
+						{{if gt (len $comment) 0}}<p class="text light grey">{{$comment | RenderEmoji}}</p>{{end}}
 					{{else if eq .GetOpType 11}}
 						<p class="text light grey">{{index .GetIssueInfos 1}}</p>
 					{{else if or (eq .GetOpType 12) (eq .GetOpType 13) (eq .GetOpType 14) (eq .GetOpType 15)}}

web_src/js/features/contextpopup.js

@@ -1,3 +1,4 @@
+import {htmlEscape} from 'escape-goat';
 import {svg} from '../svg.js';
 
 const {AppSubUrl} = window.config;
@@ -31,7 +32,7 @@ function issuePopup(owner, repo, index, $element) {
       if ((red * 0.299 + green * 0.587 + blue * 0.114) > 125) {
         color = '#000000';
       }
-      labels += `<div class="ui label" style="color: ${color}; background-color:#${label.color};">${label.name}</div>`;
+      labels += `<div class="ui label" style="color: ${color}; background-color:#${label.color};">${htmlEscape(label.name)}</div>`;
     }
     if (labels.length > 0) {
       labels = `<p>${labels}</p>`;
@@ -64,9 +65,9 @@ function issuePopup(owner, repo, index, $element) {
       },
       html: `
 <div>
-  <p><small>${issue.repository.full_name} on ${createdAt}</small></p>
+  <p><small>${htmlEscape(issue.repository.full_name)} on ${createdAt}</small></p>
-  <p><span class="${color}">${svg(octicon)}</span> <strong>${issue.title}</strong> #${index}</p>
+  <p><span class="${color}">${svg(octicon)}</span> <strong>${htmlEscape(issue.title)}</strong> #${index}</p>
-  <p>${body}</p>
+  <p>${htmlEscape(body)}</p>
   ${labels}
 </div>
 `