1.13.0 Changelog (#13782)

Co-authored-by: 6543 <6543@obermui.de>

.drone.yml

@@ -113,12 +113,6 @@ services:
     environment:
       MYSQL_ALLOW_EMPTY_PASSWORD: yes
       MYSQL_DATABASE: test
-      GOPROXY: off
-      TAGS: bindata sqlite sqlite_unlock_notify
-      GITLAB_READ_TOKEN:
-        from_secret: gitlab_read_token
-    depends_on:
-      - build
 
   - name: mysql8
     pull: default
@@ -672,7 +666,6 @@ steps:
       event:
         exclude:
         - pull_request
-
 ---
 kind: pipeline
 name: docker-linux-arm64-dry-run
@@ -702,6 +695,9 @@ steps:
       tags: linux-arm64
       build_args:
         - GOPROXY=off
+    environment:
+      PLUGIN_MIRROR:
+        from_secret: plugin_mirror
     when:
       event:
         - pull_request
@@ -746,11 +742,13 @@ steps:
         from_secret: docker_password
       username:
         from_secret: docker_username
+    environment:
+      PLUGIN_MIRROR:
+        from_secret: plugin_mirror
     when:
       event:
         exclude:
         - pull_request
-
 ---
 kind: pipeline
 name: docker-manifest

CHANGELOG.md

@@ -4,14 +4,17 @@ 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.0-RC1](https://github.com/go-gitea/gitea/releases/tag/v1.13.0-RC1) - 2020-10-14
+## [1.13.0](https://github.com/go-gitea/gitea/releases/tag/v1.13.0) - 2020-12-01
-
 * SECURITY
+  * Add Allow-/Block-List for Migrate & Mirrors (#13610) (#13776)
+  * Prevent git operations for inactive users (#13527) (#13536)
+  * Disallow urlencoded new lines in git protocol paths if there is a port (#13521) (#13524)
   * Mitigate Security vulnerability in the git hook feature (#13058)
   * Disable DSA ssh keys by default (#13056)
   * Set TLS minimum version to 1.2 (#12689)
   * Use argon as default password hash algorithm (#12688)
 * BREAKING
+  * Set RUN_MODE prod by default (#13765) (#13767)
   * Don't replace underscores in auto-generated IDs in goldmark (#12805)
   * Add Primary Key to Topic and RepoTopic tables (#12639)
   * Disable password complexity check default (#12557)
@@ -71,6 +74,40 @@ been added to each release, please refer to the [blog](https://blog.gitea.io).
   * Add endpoint for Branch Creation (#11607)
   * Add pagination headers on endpoints that support total count from database (#11145)
 * BUGFIXES
+  * Fix bogus http requests on diffs (#13760) (#13761)
+  * Show 'owner' tag for real owner (#13689) (#13743)
+  * Validate email before inserting/updating (#13475) (#13666)
+  * Fix issue/pull request list assignee filter (#13647) (#13651)
+  * Gitlab migration support for subdirectories (#13563) (#13591)
+  * Fix logic for preferred license setting (#13550) (#13557)
+  * Add missed sync branch/tag webhook (#13538) (#13556)
+  * Migration won't fail on non-migrated reactions (#13507)
+  * Fix Italian language file parsing error (#13156)
+  * Show outdated comments in pull request (#13148) (#13162)
+  * Fix parsing of pre-release git version (#13169) (#13172)
+  * Fix diff skipping lines (#13154) (#13155)
+  * When handling errors in storageHandler check underlying error (#13178) (#13193)
+  * Fix size and clickable area on file table back link (#13205) (#13207)
+  * Add better error checking for inline html diff code (#13251)
+  * Fix initial commit page & binary munching problem (#13249) (#13258)
+  * Fix migrations from remote Gitea instances when configuration not set (#13229) (#13273)
+  * Store task errors following migrations and display them (#13246) (#13287)
+  * Fix bug isEnd detection on getIssues/getPullRequests (#13299) (#13301)
+  * When the git ref is unable to be found return broken pr (#13218) (#13303)
+  * Ensure topics added using the API are added to the repository (#13285) (#13302)
+  * Fix avatar autogeneration (#13233) (#13282)
+  * Add migrated pulls to pull request task queue (#13331) (#13334)
+  * Issue comment reactions should also check pull type on API (#13349) (#13350)
+  * Fix links to repositories in /user/setting/repos (#13360) (#13362)
+  * Remove obsolete change of email on profile page (#13341) (#13347)
+  * Fix scrolling to resolved comment anchors (#13343) (#13371)
+  * Storage configuration support `[storage]` (#13314) (#13379)
+  * When creating line diffs do not split within an html entity (#13357) (#13375) (#13425) (#13427)
+  * Fix reactions on code comments (#13390) (#13401) 
+  * Add missing full names when DEFAULT_SHOW_FULL_NAME is enabled (#13424)
+  * Replies to outdated code comments should also be outdated (#13217) (#13433)
+  * Fix panic bug in handling multiple references in commit (#13486) (#13487)
+  * Prevent panic on git blame by limiting lines to 4096 bytes at most (#13470) (#13491)
   * Show original author's reviews on pull summary box (#13127)
   * Update golangci-lint to version 1.31.0 (#13102)
   * Fix line break for MS teams webhook (#13081)
@@ -140,6 +177,10 @@ been added to each release, please refer to the [blog](https://blog.gitea.io).
   * Fix Enter not working in SimpleMDE (#11564)
   * Fix bug about can't skip commits base on base branch (#11555)
 * ENHANCEMENTS
+  * Only Return JSON for responses (#13511) (#13565)
+  * Use existing analyzer module for language detection for highlighting (#13522) (#13551)
+  * Return the full rejection message and errors in flash errors (#13221) (#13237)
+  * Remove PAM from auth dropdown when unavailable (#13276) (#13281)
   * Add HostCertificate to sshd_config in Docker image (#13143)
   * Save TimeStamps for Star, Label, Follow, Watch and Collaboration to Database (#13124)
   * Improve error feedback for duplicate deploy keys (#13112)

Makefile

@@ -638,8 +638,8 @@ fomantic: $(FOMANTIC_DEST)
 
 $(FOMANTIC_DEST): $(FOMANTIC_CONFIGS) | node_modules
 	rm -rf $(FOMANTIC_DEST_DIR)
-	cp web_src/fomantic/theme.config.less node_modules/fomantic-ui/src/theme.config
+	cp -f web_src/fomantic/theme.config.less node_modules/fomantic-ui/src/theme.config
-	cp -r web_src/fomantic/_site/* node_modules/fomantic-ui/src/_site/
+	cp -fr web_src/fomantic/_site/* node_modules/fomantic-ui/src/_site/
 	npx gulp -f node_modules/fomantic-ui/gulpfile.js build
 	@touch $(FOMANTIC_DEST)
 

custom/conf/app.example.ini

@@ -8,8 +8,8 @@
 APP_NAME = Gitea: Git with a cup of tea
 ; Change it if you run locally
 RUN_USER = git
-; Either "dev", "prod" or "test", default is "dev"
+; Application run mode, affects performance and debugging. Either "dev", "prod" or "test", default is "prod"
-RUN_MODE = dev
+RUN_MODE = prod
 
 [project]
 ; Default templates for project boards
@@ -1188,6 +1188,14 @@ QUEUE_CONN_STR = "addrs=127.0.0.1:6379 db=0"
 MAX_ATTEMPTS = 3
 ; Backoff time per http/https request retry (seconds)
 RETRY_BACKOFF = 3
+; Allowed domains for migrating, default is blank. Blank means everything will be allowed.
+; Multiple domains could be separated by commas.
+ALLOWED_DOMAINS =
+; Blocklist for migrating, default is blank. Multiple domains could be separated by commas.
+; When ALLOWED_DOMAINS is not blank, this option will be ignored.
+BLOCKED_DOMAINS =
+; Allow private addresses defined by RFC 1918, RFC 1122, RFC 4632 and RFC 4291 (false by default)
+ALLOW_LOCALNETWORKS = false
 
 ; default storage for attachments, lfs and avatars
 [storage]

docker/root/etc/s6/gitea/setup

@@ -25,7 +25,7 @@ if [ ! -f ${GITEA_CUSTOM}/conf/app.ini ]; then
 
     # Substitude the environment variables in the template
     APP_NAME=${APP_NAME:-"Gitea: Git with a cup of tea"} \
-    RUN_MODE=${RUN_MODE:-"dev"} \
+    RUN_MODE=${RUN_MODE:-"prod"} \
     DOMAIN=${DOMAIN:-"localhost"} \
     SSH_DOMAIN=${SSH_DOMAIN:-"localhost"} \
     HTTP_PORT=${HTTP_PORT:-"3000"} \

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

@@ -36,9 +36,7 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`.
 - `APP_NAME`: **Gitea: Git with a cup of tea**: Application name, used in the page title.
 - `RUN_USER`: **git**: The user Gitea will run as. This should be a dedicated system
    (non-user) account. Setting this incorrectly will cause Gitea to not start.
-- `RUN_MODE`: **dev**: For performance and other purposes, change this to `prod` when
+- `RUN_MODE`: **prod**: Application run mode, affects performance and debugging. Either "dev", "prod" or "test".
-   deployed to a production environment. The installation process will set this to `prod`
-   automatically. \[prod, dev, test\]
 
 ## Repository (`repository`)
 
@@ -813,6 +811,9 @@ Task queue configuration has been moved to `queue.task`. However, the below conf
 
 - `MAX_ATTEMPTS`: **3**: Max attempts per http/https request on migrations.
 - `RETRY_BACKOFF`: **3**: Backoff time per http/https request retry (seconds)
+- `ALLOWED_DOMAINS`: **\<empty\>**: Domains allowlist for migrating repositories, default is blank. It means everything will be allowed. Multiple domains could be separated by commas.
+- `BLOCKED_DOMAINS`: **\<empty\>**: Domains blocklist for migrating repositories, default is blank. Multiple domains could be separated by commas. When `ALLOWED_DOMAINS` is not blank, this option will be ignored.
+- `ALLOW_LOCALNETWORKS`: **false**: Allow private addresses defined by RFC 1918, RFC 1122, RFC 4632 and RFC 4291
 
 ## Mirror (`mirror`)
 

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

@@ -313,6 +313,9 @@ IS_INPUT_FILE = false
 
 - `MAX_ATTEMPTS`: **3**: 在迁移过程中的 http/https 请求重试次数。
 - `RETRY_BACKOFF`: **3**: 等待下一次重试的时间，单位秒。
+- `ALLOWED_DOMAINS`: **\<empty\>**: 迁移仓库的域名白名单，默认为空，表示允许从任意域名迁移仓库，多个域名用逗号分隔。
+- `BLOCKED_DOMAINS`: **\<empty\>**: 迁移仓库的域名黑名单，默认为空，多个域名用逗号分隔。如果 `ALLOWED_DOMAINS` 不为空，此选项将会被忽略。
+- `ALLOW_LOCALNETWORKS`: **false**: Allow private addresses defined by RFC 1918
 
 ## LFS (`lfs`)
 

docs/content/doc/features/webhooks.en-us.md

@@ -30,6 +30,8 @@ All event pushes are POST requests. The methods currently supported are:
 
 ### Event information
 
+**WARNING**: The `secret` field in the payload is deprecated as of Gitea 1.13.0 and will be removed in 1.14.0: https://github.com/go-gitea/gitea/issues/11755
+
 The following is an example of event information that will be sent by Gitea to
 a Payload URL:
 

docs/content/doc/installation/with-docker.en-us.md

@@ -257,7 +257,7 @@ You can configure some of Gitea's settings via environment variables:
 (Default values are provided in **bold**)
 
 * `APP_NAME`: **"Gitea: Git with a cup of tea"**: Application name, used in the page title.
-* `RUN_MODE`: **dev**: For performance and other purposes, change this to `prod` when deployed to a production environment.
+* `RUN_MODE`: **prod**: Application run mode, affects performance and debugging. Either "dev", "prod" or "test".
 * `DOMAIN`: **localhost**: Domain name of this server, used for the displayed http clone URL in Gitea's UI.
 * `SSH_DOMAIN`: **localhost**: Domain name of this server, used for the displayed ssh clone URL in Gitea's UI. If the install page is enabled, SSH Domain Server takes DOMAIN value in the form (which overwrite this setting on save).
 * `SSH_PORT`: **22**: SSH port displayed in clone URL.

go.mod

@@ -117,10 +117,10 @@ require (
 	gopkg.in/ini.v1 v1.61.0
 	gopkg.in/ldap.v3 v3.0.2
 	gopkg.in/yaml.v2 v2.3.0
-	mvdan.cc/xurls/v2 v2.1.0
+	mvdan.cc/xurls/v2 v2.2.0
 	strk.kbt.io/projects/go/libravatar v0.0.0-20191008002943-06d1c002b251
 	xorm.io/builder v0.3.7
 	xorm.io/xorm v1.0.5
 )
 
-replace github.com/hashicorp/go-version => github.com/6543/go-version v1.2.3
+replace github.com/hashicorp/go-version => github.com/6543/go-version v1.2.4

go.sum

@@ -48,8 +48,8 @@ gitea.com/macaron/toolbox v0.0.0-20190822013122-05ff0fc766b7 h1:N9QFoeNsUXLhl14m
 gitea.com/macaron/toolbox v0.0.0-20190822013122-05ff0fc766b7/go.mod h1:kgsbFPPS4P+acDYDOPDa3N4IWWOuDJt5/INKRUz7aks=
 gitea.com/xorm/sqlfiddle v0.0.0-20180821085327-62ce714f951a h1:lSA0F4e9A2NcQSqGqTOXqu2aRi/XEQxDCBwM8yJtE6s=
 gitea.com/xorm/sqlfiddle v0.0.0-20180821085327-62ce714f951a/go.mod h1:EXuID2Zs0pAQhH8yz+DNjUbjppKQzKFAn28TMYPB6IU=
-github.com/6543/go-version v1.2.3 h1:uF30BawMhoQLzqBeCwhFcWM6HVxlzMHe/zXbzJeKP+o=
+github.com/6543/go-version v1.2.4 h1:MPsSnqNrM0HwA9tnmWNnsMdQMg4/u4fflARjwomoof4=
-github.com/6543/go-version v1.2.3/go.mod h1:fcfWh4zkneEgGXe8JJptiGwp8l6JgJJgS7oTw6P83So=
+github.com/6543/go-version v1.2.4/go.mod h1:oqFAHCwtLVUTLdhQmVZWYvaHXTdsbB4SY85at64SQEo=
 github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
@@ -768,6 +768,7 @@ github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6So
 github.com/rogpeppe/go-internal v1.1.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
 github.com/rogpeppe/go-internal v1.2.2/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
 github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
+github.com/rogpeppe/go-internal v1.5.2/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=
 github.com/rs/xid v1.2.1 h1:mhH9Nq+C1fY2l1XIpgxIiUOfNpRBYH1kKcr+qfKgjRc=
 github.com/rs/xid v1.2.1/go.mod h1:+uKXf+4Djp6Md1KODXJxgGQPKngRmWyn10oCKFzNHOQ=
 github.com/rs/zerolog v1.13.0/go.mod h1:YbFCdg8HfsridGWAh22vktObvhZbQsZXe4/zB0OKkWU=
@@ -1196,8 +1197,7 @@ honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWh
 honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
-mvdan.cc/xurls/v2 v2.1.0 h1:KaMb5GLhlcSX+e+qhbRJODnUUBvlw01jt4yrjFIHAuA=
+mvdan.cc/xurls/v2 v2.2.0/go.mod h1:EV1RMtya9D6G5DMYPGD8zTQzaHet6Jh8gFlRgGRJeO8=
-mvdan.cc/xurls/v2 v2.1.0/go.mod h1:5GrSd9rOnKOpZaji1OZLYL/yeAAtGDlo/cFe+8K5n8E=
 rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=
 strk.kbt.io/projects/go/libravatar v0.0.0-20191008002943-06d1c002b251 h1:mUcz5b3FJbP5Cvdq7Khzn6J9OCUQJaBwgBkCR+MOwSs=
 strk.kbt.io/projects/go/libravatar v0.0.0-20191008002943-06d1c002b251/go.mod h1:FJGmPh3vz9jSos1L/F91iAgnC/aejc0wIIrF2ZwJxdY=

integrations/api_admin_test.go

@@ -144,3 +144,22 @@ func TestAPIListUsersNonAdmin(t *testing.T) {
 	req := NewRequestf(t, "GET", "/api/v1/admin/users?token=%s", token)
 	session.MakeRequest(t, req, http.StatusForbidden)
 }
+
+func TestAPICreateUserInvalidEmail(t *testing.T) {
+	defer prepareTestEnv(t)()
+	adminUsername := "user1"
+	session := loginUser(t, adminUsername)
+	token := getTokenForLoggedInUser(t, session)
+	urlStr := fmt.Sprintf("/api/v1/admin/users?token=%s", token)
+	req := NewRequestWithValues(t, "POST", urlStr, map[string]string{
+		"email":                "invalid_email@domain.com\r\n",
+		"full_name":            "invalid user",
+		"login_name":           "invalidUser",
+		"must_change_password": "true",
+		"password":             "password",
+		"send_notify":          "true",
+		"source_id":            "0",
+		"username":             "invalidUser",
+	})
+	session.MakeRequest(t, req, http.StatusUnprocessableEntity)
+}

integrations/api_helper_for_declarative_test.go

@@ -5,14 +5,17 @@
 package integrations
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"net/http"
 	"testing"
+	"time"
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/auth"
+	"code.gitea.io/gitea/modules/queue"
 	api "code.gitea.io/gitea/modules/structs"
 
 	"github.com/stretchr/testify/assert"
@@ -225,11 +228,29 @@ func doAPIMergePullRequest(ctx APITestContext, owner, repo string, index int64)
 			Do:                string(models.MergeStyleMerge),
 		})
 
-		if ctx.ExpectedCode != 0 {
+		resp := ctx.Session.MakeRequest(t, req, NoExpectedStatus)
-			ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
+
-			return
+		if resp.Code == http.StatusMethodNotAllowed {
+			err := api.APIError{}
+			DecodeJSON(t, resp, &err)
+			assert.EqualValues(t, "Please try again later", err.Message)
+			queue.GetManager().FlushAll(context.Background(), 5*time.Second)
+			req = NewRequestWithJSON(t, http.MethodPost, urlStr, &auth.MergePullRequestForm{
+				MergeMessageField: "doAPIMergePullRequest Merge",
+				Do:                string(models.MergeStyleMerge),
+			})
+			resp = ctx.Session.MakeRequest(t, req, NoExpectedStatus)
+		}
+
+		expected := ctx.ExpectedCode
+		if expected == 0 {
+			expected = 200
+		}
+
+		if !assert.EqualValues(t, expected, resp.Code,
+			"Request: %s %s", req.Method, req.URL.String()) {
+			logUnexpectedResponse(t, resp)
 		}
-		ctx.Session.MakeRequest(t, req, 200)
 	}
 }
 

integrations/api_repo_test.go

@@ -309,6 +309,8 @@ func TestAPIRepoMigrate(t *testing.T) {
 		{ctxUserID: 2, userID: 1, cloneURL: "https://github.com/go-gitea/test_repo.git", repoName: "git-bad", expectedStatus: http.StatusForbidden},
 		{ctxUserID: 2, userID: 3, cloneURL: "https://github.com/go-gitea/test_repo.git", repoName: "git-org", expectedStatus: http.StatusCreated},
 		{ctxUserID: 2, userID: 6, cloneURL: "https://github.com/go-gitea/test_repo.git", repoName: "git-bad-org", expectedStatus: http.StatusForbidden},
+		{ctxUserID: 2, userID: 3, cloneURL: "https://localhost:3000/user/test_repo.git", repoName: "local-ip", expectedStatus: http.StatusUnprocessableEntity},
+		{ctxUserID: 2, userID: 3, cloneURL: "https://10.0.0.1/user/test_repo.git", repoName: "private-ip", expectedStatus: http.StatusUnprocessableEntity},
 	}
 
 	defer prepareTestEnv(t)()
@@ -325,8 +327,16 @@ func TestAPIRepoMigrate(t *testing.T) {
 		if resp.Code == http.StatusUnprocessableEntity {
 			respJSON := map[string]string{}
 			DecodeJSON(t, resp, &respJSON)
-			if assert.Equal(t, respJSON["message"], "Remote visit addressed rate limitation.") {
+			switch respJSON["message"] {
+			case "Remote visit addressed rate limitation.":
 				t.Log("test hit github rate limitation")
+			case "migrate from '10.0.0.1' is not allowed: the host resolve to a private ip address '10.0.0.1'":
+				assert.EqualValues(t, "private-ip", testCase.repoName)
+			case "migrate from 'localhost:3000' is not allowed: the host resolve to a private ip address '::1'",
+				"migrate from 'localhost:3000' is not allowed: the host resolve to a private ip address '127.0.0.1'":
+				assert.EqualValues(t, "local-ip", testCase.repoName)
+			default:
+				t.Errorf("unexpected error '%v' on url '%s'", respJSON["message"], testCase.cloneURL)
 			}
 		} else {
 			assert.EqualValues(t, testCase.expectedStatus, resp.Code)

integrations/api_user_heatmap_test.go

@@ -26,7 +26,7 @@ func TestUserHeatmap(t *testing.T) {
 	var heatmap []*models.UserHeatmapData
 	DecodeJSON(t, resp, &heatmap)
 	var dummyheatmap []*models.UserHeatmapData
-	dummyheatmap = append(dummyheatmap, &models.UserHeatmapData{Timestamp: 1571616000, Contributions: 1})
+	dummyheatmap = append(dummyheatmap, &models.UserHeatmapData{Timestamp: 1603152000, Contributions: 1})
 
 	assert.Equal(t, dummyheatmap, heatmap)
 }

integrations/auth_ldap_test.go

@@ -141,7 +141,7 @@ func TestLDAPUserSignin(t *testing.T) {
 
 	assert.Equal(t, u.UserName, htmlDoc.GetInputValueByName("name"))
 	assert.Equal(t, u.FullName, htmlDoc.GetInputValueByName("full_name"))
-	assert.Equal(t, u.Email, htmlDoc.GetInputValueByName("email"))
+	assert.Equal(t, u.Email, htmlDoc.Find(`label[for="email"]`).Siblings().First().Text())
 }
 
 func TestLDAPUserSync(t *testing.T) {

integrations/html_helper.go

@@ -37,6 +37,13 @@ func (doc *HTMLDoc) GetInputValueByName(name string) string {
 	return text
 }
 
+// Find gets the descendants of each element in the current set of
+// matched elements, filtered by a selector. It returns a new Selection
+// object containing these matched elements.
+func (doc *HTMLDoc) Find(selector string) *goquery.Selection {
+	return doc.doc.Find(selector)
+}
+
 // GetCSRF for get CSRC token value from input
 func (doc *HTMLDoc) GetCSRF() string {
 	return doc.GetInputValueByName("_csrf")

integrations/integration_test.go

@@ -11,7 +11,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"log"
 	"net/http"
 	"net/http/cookiejar"
 	"net/http/httptest"
@@ -27,8 +26,10 @@ import (
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/graceful"
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/queue"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/storage"
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/routers"
 	"code.gitea.io/gitea/routers/routes"
@@ -59,6 +60,8 @@ func NewNilResponseRecorder() *NilResponseRecorder {
 }
 
 func TestMain(m *testing.M) {
+	defer log.Close()
+
 	managerCtx, cancel := context.WithCancel(context.Background())
 	graceful.InitManager(managerCtx)
 	defer cancel()
@@ -142,6 +145,10 @@ func initIntegrationTest() {
 	util.RemoveAll(models.LocalCopyPath())
 	setting.CheckLFSVersion()
 	setting.InitDBConfig()
+	if err := storage.Init(); err != nil {
+		fmt.Printf("Init storage failed: %v", err)
+		os.Exit(1)
+	}
 
 	switch {
 	case setting.Database.UseMySQL:
@@ -149,27 +156,27 @@ func initIntegrationTest() {
 			setting.Database.User, setting.Database.Passwd, setting.Database.Host))
 		defer db.Close()
 		if err != nil {
-			log.Fatalf("sql.Open: %v", err)
+			log.Fatal("sql.Open: %v", err)
 		}
 		if _, err = db.Exec(fmt.Sprintf("CREATE DATABASE IF NOT EXISTS %s", setting.Database.Name)); err != nil {
-			log.Fatalf("db.Exec: %v", err)
+			log.Fatal("db.Exec: %v", err)
 		}
 	case setting.Database.UsePostgreSQL:
 		db, err := sql.Open("postgres", fmt.Sprintf("postgres://%s:%s@%s/?sslmode=%s",
 			setting.Database.User, setting.Database.Passwd, setting.Database.Host, setting.Database.SSLMode))
 		defer db.Close()
 		if err != nil {
-			log.Fatalf("sql.Open: %v", err)
+			log.Fatal("sql.Open: %v", err)
 		}
 		dbrows, err := db.Query(fmt.Sprintf("SELECT 1 FROM pg_database WHERE datname = '%s'", setting.Database.Name))
 		if err != nil {
-			log.Fatalf("db.Query: %v", err)
+			log.Fatal("db.Query: %v", err)
 		}
 		defer dbrows.Close()
 
 		if !dbrows.Next() {
 			if _, err = db.Exec(fmt.Sprintf("CREATE DATABASE %s", setting.Database.Name)); err != nil {
-				log.Fatalf("db.Exec: CREATE DATABASE: %v", err)
+				log.Fatal("db.Exec: CREATE DATABASE: %v", err)
 			}
 		}
 		// Check if we need to setup a specific schema
@@ -183,18 +190,18 @@ func initIntegrationTest() {
 		// This is a different db object; requires a different Close()
 		defer db.Close()
 		if err != nil {
-			log.Fatalf("sql.Open: %v", err)
+			log.Fatal("sql.Open: %v", err)
 		}
 		schrows, err := db.Query(fmt.Sprintf("SELECT 1 FROM information_schema.schemata WHERE schema_name = '%s'", setting.Database.Schema))
 		if err != nil {
-			log.Fatalf("db.Query: %v", err)
+			log.Fatal("db.Query: %v", err)
 		}
 		defer schrows.Close()
 
 		if !schrows.Next() {
 			// Create and setup a DB schema
 			if _, err = db.Exec(fmt.Sprintf("CREATE SCHEMA %s", setting.Database.Schema)); err != nil {
-				log.Fatalf("db.Exec: CREATE SCHEMA: %v", err)
+				log.Fatal("db.Exec: CREATE SCHEMA: %v", err)
 			}
 		}
 
@@ -203,10 +210,10 @@ func initIntegrationTest() {
 		db, err := sql.Open("mssql", fmt.Sprintf("server=%s; port=%s; database=%s; user id=%s; password=%s;",
 			host, port, "master", setting.Database.User, setting.Database.Passwd))
 		if err != nil {
-			log.Fatalf("sql.Open: %v", err)
+			log.Fatal("sql.Open: %v", err)
 		}
 		if _, err := db.Exec(fmt.Sprintf("If(db_id(N'%s') IS NULL) BEGIN CREATE DATABASE %s; END;", setting.Database.Name, setting.Database.Name)); err != nil {
-			log.Fatalf("db.Exec: %v", err)
+			log.Fatal("db.Exec: %v", err)
 		}
 		defer db.Close()
 	}

integrations/lfs_getobject_test.go

@@ -78,6 +78,7 @@ func storeAndGetLfs(t *testing.T, content *[]byte, extraHeader *http.Header, exp
 			}
 		}
 	}
+
 	resp := session.MakeRequest(t, req, expectedStatus)
 
 	return resp
@@ -210,7 +211,7 @@ func TestGetLFSRange(t *testing.T) {
 		{"bytes=0-10", "123456789\n", http.StatusPartialContent},
 		// end-range bigger than length-1 is ignored
 		{"bytes=0-11", "123456789\n", http.StatusPartialContent},
-		{"bytes=11-", "", http.StatusPartialContent},
+		{"bytes=11-", "Requested Range Not Satisfiable", http.StatusRequestedRangeNotSatisfiable},
 		// incorrect header value cause whole header to be ignored
 		{"bytes=-", "123456789\n", http.StatusOK},
 		{"foobar", "123456789\n", http.StatusOK},

integrations/mysql.ini.tmpl

@@ -45,19 +45,21 @@ START_SSH_SERVER = true
 OFFLINE_MODE     = false
 
 LFS_START_SERVER = true
-LFS_CONTENT_PATH = integrations/gitea-integration-mysql/datalfs-mysql
 LFS_JWT_SECRET   = Tv_MjmZuHqpIY6GFl12ebgkRAMt4RlWt0v4EHKSXO0w
-LFS_STORE_TYPE = minio
+
-LFS_SERVE_DIRECT = false
+[lfs]
-LFS_MINIO_ENDPOINT = minio:9000
+MINIO_BASE_PATH = lfs/
-LFS_MINIO_ACCESS_KEY_ID = 123456
-LFS_MINIO_SECRET_ACCESS_KEY = 12345678
-LFS_MINIO_BUCKET = gitea
-LFS_MINIO_LOCATION = us-east-1
-LFS_MINIO_BASE_PATH = lfs/
-LFS_MINIO_USE_SSL = false
 
 [attachment]
+MINIO_BASE_PATH = attachments/
+
+[avatars]
+MINIO_BASE_PATH = avatars/
+
+[repo-avatars]
+MINIO_BASE_PATH = repo-avatars/
+
+[storage]
 STORAGE_TYPE = minio
 SERVE_DIRECT = false
 MINIO_ENDPOINT = minio:9000
@@ -65,7 +67,6 @@ MINIO_ACCESS_KEY_ID = 123456
 MINIO_SECRET_ACCESS_KEY = 12345678
 MINIO_BUCKET = gitea
 MINIO_LOCATION = us-east-1
-MINIO_BASE_PATH = attachments/
 MINIO_USE_SSL = false
 
 [mailer]
@@ -88,9 +89,6 @@ ENABLE_NOTIFY_MAIL                = true
 DISABLE_GRAVATAR              = false
 ENABLE_FEDERATED_AVATAR       = false
 
-AVATAR_UPLOAD_PATH            = integrations/gitea-integration-mysql/data/avatars
-REPOSITORY_AVATAR_UPLOAD_PATH = integrations/gitea-integration-mysql/data/repo-avatars
-
 [session]
 PROVIDER        = file
 PROVIDER_CONFIG = integrations/gitea-integration-mysql/data/sessions

integrations/signup_test.go

@@ -5,10 +5,14 @@
 package integrations
 
 import (
+	"fmt"
 	"net/http"
+	"strings"
 	"testing"
 
 	"code.gitea.io/gitea/modules/setting"
+	"github.com/stretchr/testify/assert"
+	"github.com/unknwon/i18n"
 )
 
 func TestSignup(t *testing.T) {
@@ -28,3 +32,37 @@ func TestSignup(t *testing.T) {
 	req = NewRequest(t, "GET", "/exampleUser")
 	MakeRequest(t, req, http.StatusOK)
 }
+
+func TestSignupEmail(t *testing.T) {
+	defer prepareTestEnv(t)()
+
+	setting.Service.EnableCaptcha = false
+
+	tests := []struct {
+		email      string
+		wantStatus int
+		wantMsg    string
+	}{
+		{"exampleUser@example.com\r\n", http.StatusOK, i18n.Tr("en", "form.email_invalid", nil)},
+		{"exampleUser@example.com\r", http.StatusOK, i18n.Tr("en", "form.email_invalid", nil)},
+		{"exampleUser@example.com\n", http.StatusOK, i18n.Tr("en", "form.email_invalid", nil)},
+		{"exampleUser@example.com", http.StatusFound, ""},
+	}
+
+	for i, test := range tests {
+		req := NewRequestWithValues(t, "POST", "/user/sign_up", map[string]string{
+			"user_name": fmt.Sprintf("exampleUser%d", i),
+			"email":     test.email,
+			"password":  "examplePassword!1",
+			"retype":    "examplePassword!1",
+		})
+		resp := MakeRequest(t, req, test.wantStatus)
+		if test.wantMsg != "" {
+			htmlDoc := NewHTMLParser(t, resp.Body)
+			assert.Equal(t,
+				test.wantMsg,
+				strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()),
+			)
+		}
+	}
+}

models/error.go

@@ -193,6 +193,21 @@ func (err ErrEmailAlreadyUsed) Error() string {
 	return fmt.Sprintf("e-mail already in use [email: %s]", err.Email)
 }
 
+// ErrEmailInvalid represents an error where the email address does not comply with RFC 5322
+type ErrEmailInvalid struct {
+	Email string
+}
+
+// IsErrEmailInvalid checks if an error is an ErrEmailInvalid
+func IsErrEmailInvalid(err error) bool {
+	_, ok := err.(ErrEmailInvalid)
+	return ok
+}
+
+func (err ErrEmailInvalid) Error() string {
+	return fmt.Sprintf("e-mail invalid [email: %s]", err.Email)
+}
+
 // ErrOpenIDAlreadyUsed represents a "OpenIDAlreadyUsed" kind of error.
 type ErrOpenIDAlreadyUsed struct {
 	OpenID string
@@ -1004,6 +1019,29 @@ func IsErrWontSign(err error) bool {
 	return ok
 }
 
+// ErrMigrationNotAllowed explains why a migration from an url is not allowed
+type ErrMigrationNotAllowed struct {
+	Host          string
+	NotResolvedIP bool
+	PrivateNet    string
+}
+
+func (e *ErrMigrationNotAllowed) Error() string {
+	if e.NotResolvedIP {
+		return fmt.Sprintf("migrate from '%s' is not allowed: unknown hostname", e.Host)
+	}
+	if len(e.PrivateNet) != 0 {
+		return fmt.Sprintf("migrate from '%s' is not allowed: the host resolve to a private ip address '%s'", e.Host, e.PrivateNet)
+	}
+	return fmt.Sprintf("migrate from '%s is not allowed'", e.Host)
+}
+
+// IsErrMigrationNotAllowed checks if an error is a ErrMigrationNotAllowed
+func IsErrMigrationNotAllowed(err error) bool {
+	_, ok := err.(*ErrMigrationNotAllowed)
+	return ok
+}
+
 // __________                             .__
 // \______   \____________    ____   ____ |  |__
 //  |    |  _/\_  __ \__  \  /    \_/ ___\|  |  \
@@ -2003,7 +2041,7 @@ type ErrNotValidReviewRequest struct {
 
 // IsErrNotValidReviewRequest checks if an error is a ErrNotValidReviewRequest.
 func IsErrNotValidReviewRequest(err error) bool {
-	_, ok := err.(ErrReviewNotExist)
+	_, ok := err.(ErrNotValidReviewRequest)
 	return ok
 }
 

models/fixtures/action.yml

@@ -5,7 +5,7 @@
   act_user_id: 2
   repo_id: 2
   is_private: true
-  created_unix: 1571686356
+  created_unix: 1603228283
 
 -
   id: 2

models/issue_comment.go

@@ -725,6 +725,7 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
 		RefAction:        opts.RefAction,
 		RefIsPull:        opts.RefIsPull,
 		IsForcePush:      opts.IsForcePush,
+		Invalidated:      opts.Invalidated,
 	}
 	if _, err = e.Insert(comment); err != nil {
 		return nil, err
@@ -891,6 +892,7 @@ type CreateCommentOptions struct {
 	RefAction        references.XRefAction
 	RefIsPull        bool
 	IsForcePush      bool
+	Invalidated      bool
 }
 
 // CreateComment creates comment of issue or commit.
@@ -966,6 +968,8 @@ type FindCommentsOptions struct {
 	ReviewID int64
 	Since    int64
 	Before   int64
+	Line     int64
+	TreePath string
 	Type     CommentType
 }
 
@@ -989,6 +993,12 @@ func (opts *FindCommentsOptions) toConds() builder.Cond {
 	if opts.Type != CommentTypeUnknown {
 		cond = cond.And(builder.Eq{"comment.type": opts.Type})
 	}
+	if opts.Line > 0 {
+		cond = cond.And(builder.Eq{"comment.line": opts.Line})
+	}
+	if len(opts.TreePath) > 0 {
+		cond = cond.And(builder.Eq{"comment.tree_path": opts.TreePath})
+	}
 	return cond
 }
 
@@ -1003,6 +1013,8 @@ func findComments(e Engine, opts FindCommentsOptions) ([]*Comment, error) {
 		sess = opts.setSessionPagination(sess)
 	}
 
+	// WARNING: If you change this order you will need to fix createCodeComment
+
 	return comments, sess.
 		Asc("comment.created_unix").
 		Asc("comment.id").
@@ -1124,6 +1136,10 @@ func fetchCodeCommentsByReview(e Engine, issue *Issue, currentUser *User, review
 			return nil, err
 		}
 
+		if err := comment.LoadReactions(issue.Repo); err != nil {
+			return nil, err
+		}
+
 		if re, ok := reviews[comment.ReviewID]; ok && re != nil {
 			// If the review is pending only the author can see the comments (except the review is set)
 			if review.ID == 0 {

models/repo_permission.go

@@ -271,6 +271,27 @@ func getUserRepoPermission(e Engine, repo *Repository, user *User) (perm Permiss
 	return
 }
 
+// IsUserRealRepoAdmin check if this user is real repo admin
+func IsUserRealRepoAdmin(repo *Repository, user *User) (bool, error) {
+	if repo.OwnerID == user.ID {
+		return true, nil
+	}
+
+	sess := x.NewSession()
+	defer sess.Close()
+
+	if err := repo.getOwner(sess); err != nil {
+		return false, err
+	}
+
+	accessMode, err := accessLevel(sess, user, repo)
+	if err != nil {
+		return false, err
+	}
+
+	return accessMode >= AccessModeAdmin, nil
+}
+
 // IsUserRepoAdmin return true if user has admin right of a repo
 func IsUserRepoAdmin(repo *Repository, user *User) (bool, error) {
 	return isUserRepoAdmin(x, repo, user)

models/task.go

@@ -147,6 +147,27 @@ func GetMigratingTask(repoID int64) (*Task, error) {
 	return &task, nil
 }
 
+// GetMigratingTaskByID returns the migrating task by repo's id
+func GetMigratingTaskByID(id, doerID int64) (*Task, *migration.MigrateOptions, error) {
+	var task = Task{
+		ID:     id,
+		DoerID: doerID,
+		Type:   structs.TaskTypeMigrateRepo,
+	}
+	has, err := x.Get(&task)
+	if err != nil {
+		return nil, nil, err
+	} else if !has {
+		return nil, nil, ErrTaskDoesNotExist{id, 0, task.Type}
+	}
+
+	var opts migration.MigrateOptions
+	if err := json.Unmarshal([]byte(task.PayloadContent), &opts); err != nil {
+		return nil, nil, err
+	}
+	return &task, &opts, nil
+}
+
 // FindTaskOptions find all tasks
 type FindTaskOptions struct {
 	Status int

models/topic.go

@@ -197,10 +197,13 @@ func FindTopics(opts *FindTopicOptions) (topics []*Topic, err error) {
 
 // GetRepoTopicByName retrives topic from name for a repo if it exist
 func GetRepoTopicByName(repoID int64, topicName string) (*Topic, error) {
+	return getRepoTopicByName(x, repoID, topicName)
+}
+func getRepoTopicByName(e Engine, repoID int64, topicName string) (*Topic, error) {
 	var cond = builder.NewCond()
 	var topic Topic
 	cond = cond.And(builder.Eq{"repo_topic.repo_id": repoID}).And(builder.Eq{"topic.name": topicName})
-	sess := x.Table("topic").Where(cond)
+	sess := e.Table("topic").Where(cond)
 	sess.Join("INNER", "repo_topic", "repo_topic.topic_id = topic.id")
 	has, err := sess.Get(&topic)
 	if has {
@@ -211,7 +214,13 @@ func GetRepoTopicByName(repoID int64, topicName string) (*Topic, error) {
 
 // AddTopic adds a topic name to a repository (if it does not already have it)
 func AddTopic(repoID int64, topicName string) (*Topic, error) {
-	topic, err := GetRepoTopicByName(repoID, topicName)
+	sess := x.NewSession()
+	defer sess.Close()
+	if err := sess.Begin(); err != nil {
+		return nil, err
+	}
+
+	topic, err := getRepoTopicByName(sess, repoID, topicName)
 	if err != nil {
 		return nil, err
 	}
@@ -220,7 +229,25 @@ func AddTopic(repoID int64, topicName string) (*Topic, error) {
 		return topic, nil
 	}
 
-	return addTopicByNameToRepo(x, repoID, topicName)
+	topic, err = addTopicByNameToRepo(sess, repoID, topicName)
+	if err != nil {
+		return nil, err
+	}
+
+	topicNames := make([]string, 0, 25)
+	if err := sess.Select("name").Table("topic").
+		Join("INNER", "repo_topic", "repo_topic.topic_id = topic.id").
+		Where("repo_topic.repo_id = ?", repoID).Desc("topic.repo_count").Find(&topicNames); err != nil {
+		return nil, err
+	}
+
+	if _, err := sess.ID(repoID).Cols("topics").Update(&Repository{
+		Topics: topicNames,
+	}); err != nil {
+		return nil, err
+	}
+
+	return topic, sess.Commit()
 }
 
 // DeleteTopic removes a topic name from a repository (if it has it)

models/user.go

@@ -191,9 +191,6 @@ func (u *User) BeforeUpdate() {
 		if len(u.AvatarEmail) == 0 {
 			u.AvatarEmail = u.Email
 		}
-		if len(u.AvatarEmail) > 0 && u.Avatar == "" {
-			u.Avatar = base.HashEmail(u.AvatarEmail)
-		}
 	}
 
 	u.LowerName = strings.ToLower(u.Name)
@@ -824,6 +821,10 @@ func CreateUser(u *User) (err error) {
 		return ErrEmailAlreadyUsed{u.Email}
 	}
 
+	if err = ValidateEmail(u.Email); err != nil {
+		return err
+	}
+
 	isExist, err = isEmailUsed(sess, u.Email)
 	if err != nil {
 		return err
@@ -835,7 +836,6 @@ func CreateUser(u *User) (err error) {
 
 	u.LowerName = strings.ToLower(u.Name)
 	u.AvatarEmail = u.Email
-	u.Avatar = base.HashEmail(u.AvatarEmail)
 	if u.Rands, err = GetUserSalt(); err != nil {
 		return err
 	}
@@ -967,8 +967,12 @@ func checkDupEmail(e Engine, u *User) error {
 	return nil
 }
 
-func updateUser(e Engine, u *User) error {
+func updateUser(e Engine, u *User) (err error) {
-	_, err := e.ID(u.ID).AllCols().Update(u)
+	u.Email = strings.ToLower(u.Email)
+	if err = ValidateEmail(u.Email); err != nil {
+		return err
+	}
+	_, err = e.ID(u.ID).AllCols().Update(u)
 	return err
 }
 
@@ -988,13 +992,21 @@ func updateUserCols(e Engine, u *User, cols ...string) error {
 }
 
 // UpdateUserSetting updates user's settings.
-func UpdateUserSetting(u *User) error {
+func UpdateUserSetting(u *User) (err error) {
+	sess := x.NewSession()
+	defer sess.Close()
+	if err = sess.Begin(); err != nil {
+		return err
+	}
 	if !u.IsOrganization() {
-		if err := checkDupEmail(x, u); err != nil {
+		if err = checkDupEmail(sess, u); err != nil {
 			return err
 		}
 	}
-	return updateUser(x, u)
+	if err = updateUser(sess, u); err != nil {
+		return err
+	}
+	return sess.Commit()
 }
 
 // deleteBeans deletes all given beans, beans should contain delete conditions.

models/user_avatar.go

@@ -39,10 +39,9 @@ func (u *User) generateRandomAvatar(e Engine) error {
 	if err != nil {
 		return fmt.Errorf("RandomImage: %v", err)
 	}
-	// NOTICE for random avatar, it still uses id as avatar name, but custom avatar use md5
+
-	// since random image is not a user's photo, there is no security for enumable
 	if u.Avatar == "" {
-		u.Avatar = fmt.Sprintf("%d", u.ID)
+		u.Avatar = base.HashEmail(u.AvatarEmail)
 	}
 
 	if err := storage.SaveFrom(storage.Avatars, u.CustomAvatarRelativePath(), func(w io.Writer) error {

models/user_heatmap_test.go

@@ -17,7 +17,7 @@ func TestGetUserHeatmapDataByUser(t *testing.T) {
 		CountResult int
 		JSONResult  string
 	}{
-		{2, 1, `[{"timestamp":1571616000,"contributions":1}]`},
+		{2, 1, `[{"timestamp":1603152000,"contributions":1}]`},
 		{3, 0, `[]`},
 	}
 	// Prepare

models/user_mail.go

@@ -8,6 +8,7 @@ package models
 import (
 	"errors"
 	"fmt"
+	"net/mail"
 	"strings"
 
 	"code.gitea.io/gitea/modules/log"
@@ -32,6 +33,19 @@ type EmailAddress struct {
 	IsPrimary   bool `xorm:"-"`
 }
 
+// ValidateEmail check if email is a allowed address
+func ValidateEmail(email string) error {
+	if len(email) == 0 {
+		return nil
+	}
+
+	if _, err := mail.ParseAddress(email); err != nil {
+		return ErrEmailInvalid{email}
+	}
+
+	return nil
+}
+
 // GetEmailAddresses returns all email addresses belongs to given user.
 func GetEmailAddresses(uid int64) ([]*EmailAddress, error) {
 	emails := make([]*EmailAddress, 0, 5)
@@ -143,6 +157,10 @@ func addEmailAddress(e Engine, email *EmailAddress) error {
 		return ErrEmailAlreadyUsed{email.Email}
 	}
 
+	if err = ValidateEmail(email.Email); err != nil {
+		return err
+	}
+
 	_, err = e.Insert(email)
 	return err
 }
@@ -167,6 +185,9 @@ func AddEmailAddresses(emails []*EmailAddress) error {
 		} else if used {
 			return ErrEmailAlreadyUsed{emails[i].Email}
 		}
+		if err = ValidateEmail(emails[i].Email); err != nil {
+			return err
+		}
 	}
 
 	if _, err := x.Insert(emails); err != nil {

models/user_test.go

@@ -346,6 +346,21 @@ func TestCreateUser(t *testing.T) {
 	assert.NoError(t, DeleteUser(user))
 }
 
+func TestCreateUserInvalidEmail(t *testing.T) {
+	user := &User{
+		Name:               "GiteaBot",
+		Email:              "GiteaBot@gitea.io\r\n",
+		Passwd:             ";p['////..-++']",
+		IsAdmin:            false,
+		Theme:              setting.UI.DefaultTheme,
+		MustChangePassword: false,
+	}
+
+	err := CreateUser(user)
+	assert.Error(t, err)
+	assert.True(t, IsErrEmailInvalid(err))
+}
+
 func TestCreateUser_Issue5882(t *testing.T) {
 
 	// Init settings

modules/auth/pam/pam.go

@@ -12,6 +12,9 @@ import (
 	"github.com/msteinert/pam"
 )
 
+// Supported is true when built with PAM
+var Supported = true
+
 // Auth pam auth service
 func Auth(serviceName, userName, passwd string) (string, error) {
 	t, err := pam.StartFunc(serviceName, userName, func(s pam.Style, msg string) (string, error) {

modules/auth/pam/pam_stub.go

@@ -10,6 +10,9 @@ import (
 	"errors"
 )
 
+// Supported is false when built without PAM
+var Supported = false
+
 // Auth not supported lack of pam tag
 func Auth(serviceName, userName, passwd string) (string, error) {
 	return "", errors.New("PAM not supported")

modules/auth/repo_form.go

@@ -102,6 +102,9 @@ func ParseRemoteAddr(remoteAddr, authUsername, authPassword string, user *models
 			u.User = url.UserPassword(authUsername, authPassword)
 		}
 		remoteAddr = u.String()
+		if u.Scheme == "git" && u.Port() != "" && (strings.Contains(remoteAddr, "%0d") || strings.Contains(remoteAddr, "%0a")) {
+			return "", models.ErrInvalidCloneAddr{IsURLError: true}
+		}
 	} else if !user.CanImportLocal() {
 		return "", models.ErrInvalidCloneAddr{IsPermissionDenied: true}
 	} else if !com.IsDir(remoteAddr) {

modules/auth/user_form.go

@@ -199,7 +199,6 @@ func (f *AccessTokenForm) Validate(ctx *macaron.Context, errs binding.Errors) bi
 type UpdateProfileForm struct {
 	Name                string `binding:"AlphaDashDot;MaxSize(40)"`
 	FullName            string `binding:"MaxSize(100)"`
-	Email               string `binding:"Required;Email;MaxSize(254)"`
 	KeepEmailPrivate    bool
 	Website             string `binding:"ValidUrl;MaxSize(255)"`
 	Location            string `binding:"MaxSize(50)"`

modules/context/api.go

@@ -255,3 +255,61 @@ func (ctx *APIContext) NotFound(objs ...interface{}) {
 		"errors":            errors,
 	})
 }
+
+// RepoRefForAPI handles repository reference names when the ref name is not explicitly given
+func RepoRefForAPI() macaron.Handler {
+	return func(ctx *APIContext) {
+		// Empty repository does not have reference information.
+		if ctx.Repo.Repository.IsEmpty {
+			return
+		}
+
+		var err error
+
+		if ctx.Repo.GitRepo == nil {
+			repoPath := models.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
+			ctx.Repo.GitRepo, err = git.OpenRepository(repoPath)
+			if err != nil {
+				ctx.InternalServerError(err)
+				return
+			}
+			// We opened it, we should close it
+			defer func() {
+				// If it's been set to nil then assume someone else has closed it.
+				if ctx.Repo.GitRepo != nil {
+					ctx.Repo.GitRepo.Close()
+				}
+			}()
+		}
+
+		refName := getRefName(ctx.Context, RepoRefAny)
+
+		if ctx.Repo.GitRepo.IsBranchExist(refName) {
+			ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetBranchCommit(refName)
+			if err != nil {
+				ctx.InternalServerError(err)
+				return
+			}
+			ctx.Repo.CommitID = ctx.Repo.Commit.ID.String()
+		} else if ctx.Repo.GitRepo.IsTagExist(refName) {
+			ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetTagCommit(refName)
+			if err != nil {
+				ctx.InternalServerError(err)
+				return
+			}
+			ctx.Repo.CommitID = ctx.Repo.Commit.ID.String()
+		} else if len(refName) == 40 {
+			ctx.Repo.CommitID = refName
+			ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetCommit(refName)
+			if err != nil {
+				ctx.NotFound("GetCommit", err)
+				return
+			}
+		} else {
+			ctx.NotFound(fmt.Errorf("not exist: '%s'", ctx.Params("*")))
+			return
+		}
+
+		ctx.Next()
+	}
+}

modules/context/repo.go

@@ -704,7 +704,6 @@ func RepoRefByType(refType RepoRefType) macaron.Handler {
 			err     error
 		)
 
-		// For API calls.
 		if ctx.Repo.GitRepo == nil {
 			repoPath := models.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
 			ctx.Repo.GitRepo, err = git.OpenRepository(repoPath)
@@ -773,7 +772,7 @@ func RepoRefByType(refType RepoRefType) macaron.Handler {
 
 				ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetCommit(refName)
 				if err != nil {
-					ctx.NotFound("GetCommit", nil)
+					ctx.NotFound("GetCommit", err)
 					return
 				}
 			} else {

modules/git/blame.go

@@ -27,7 +27,7 @@ type BlameReader struct {
 	cmd     *exec.Cmd
 	pid     int64
 	output  io.ReadCloser
-	scanner *bufio.Scanner
+	reader  *bufio.Reader
 	lastSha *string
 	cancel  context.CancelFunc
 }
@@ -38,23 +38,30 @@ var shaLineRegex = regexp.MustCompile("^([a-z0-9]{40})")
 func (r *BlameReader) NextPart() (*BlamePart, error) {
 	var blamePart *BlamePart
 
-	scanner := r.scanner
+	reader := r.reader
 
 	if r.lastSha != nil {
 		blamePart = &BlamePart{*r.lastSha, make([]string, 0)}
 	}
 
-	for scanner.Scan() {
+	var line []byte
-		line := scanner.Text()
+	var isPrefix bool
+	var err error
+
+	for err != io.EOF {
+		line, isPrefix, err = reader.ReadLine()
+		if err != nil && err != io.EOF {
+			return blamePart, err
+		}
 
-		// Skip empty lines
 		if len(line) == 0 {
+			// isPrefix will be false
 			continue
 		}
 
-		lines := shaLineRegex.FindStringSubmatch(line)
+		lines := shaLineRegex.FindSubmatch(line)
 		if lines != nil {
-			sha1 := lines[1]
+			sha1 := string(lines[1])
 
 			if blamePart == nil {
 				blamePart = &BlamePart{sha1, make([]string, 0)}
@@ -62,12 +69,27 @@ func (r *BlameReader) NextPart() (*BlamePart, error) {
 
 			if blamePart.Sha != sha1 {
 				r.lastSha = &sha1
+				// need to munch to end of line...
+				for isPrefix {
+					_, isPrefix, err = reader.ReadLine()
+					if err != nil && err != io.EOF {
+						return blamePart, err
+					}
+				}
 				return blamePart, nil
 			}
 		} else if line[0] == '\t' {
 			code := line[1:]
 
-			blamePart.Lines = append(blamePart.Lines, code)
+			blamePart.Lines = append(blamePart.Lines, string(code))
+		}
+
+		// need to munch to end of line...
+		for isPrefix {
+			_, isPrefix, err = reader.ReadLine()
+			if err != nil && err != io.EOF {
+				return blamePart, err
+			}
 		}
 	}
 
@@ -121,13 +143,13 @@ func createBlameReader(ctx context.Context, dir string, command ...string) (*Bla
 
 	pid := process.GetManager().Add(fmt.Sprintf("GetBlame [repo_path: %s]", dir), cancel)
 
-	scanner := bufio.NewScanner(stdout)
+	reader := bufio.NewReader(stdout)
 
 	return &BlameReader{
 		cmd,
 		pid,
 		stdout,
-		scanner,
+		reader,
 		nil,
 		cancel,
 	}, nil

modules/highlight/highlight.go

@@ -13,6 +13,7 @@ import (
 	"strings"
 	"sync"
 
+	"code.gitea.io/gitea/modules/analyze"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"github.com/alecthomas/chroma/formatters/html"
@@ -117,9 +118,11 @@ func File(numLines int, fileName string, code []byte) map[int]string {
 		fileName = "test." + val
 	}
 
-	lexer := lexers.Match(fileName)
+	language := analyze.GetCodeLanguage(fileName, code)
+
+	lexer := lexers.Get(language)
 	if lexer == nil {
-		lexer = lexers.Analyse(string(code))
+		lexer = lexers.Match(fileName)
 		if lexer == nil {
 			lexer = lexers.Fallback
 		}

modules/lfs/content_store.go

@@ -8,6 +8,7 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"errors"
+	"fmt"
 	"io"
 	"os"
 
@@ -21,6 +22,21 @@ var (
 	errSizeMismatch = errors.New("Content size does not match")
 )
 
+// ErrRangeNotSatisfiable represents an error which request range is not satisfiable.
+type ErrRangeNotSatisfiable struct {
+	FromByte int64
+}
+
+func (err ErrRangeNotSatisfiable) Error() string {
+	return fmt.Sprintf("Requested range %d is not satisfiable", err.FromByte)
+}
+
+// IsErrRangeNotSatisfiable returns true if the error is an ErrRangeNotSatisfiable
+func IsErrRangeNotSatisfiable(err error) bool {
+	_, ok := err.(ErrRangeNotSatisfiable)
+	return ok
+}
+
 // ContentStore provides a simple file system based storage.
 type ContentStore struct {
 	storage.ObjectStorage
@@ -35,7 +51,12 @@ func (s *ContentStore) Get(meta *models.LFSMetaObject, fromByte int64) (io.ReadC
 		return nil, err
 	}
 	if fromByte > 0 {
-		_, err = f.Seek(fromByte, os.SEEK_CUR)
+		if fromByte >= meta.Size {
+			return nil, ErrRangeNotSatisfiable{
+				FromByte: fromByte,
+			}
+		}
+		_, err = f.Seek(fromByte, io.SeekStart)
 		if err != nil {
 			log.Error("Whilst trying to read LFS OID[%s]: Unable to seek to %d Error: %v", meta.Oid, fromByte, err)
 		}

modules/lfs/server.go

@@ -191,8 +191,12 @@ func getContentHandler(ctx *context.Context) {
 	contentStore := &ContentStore{ObjectStorage: storage.LFS}
 	content, err := contentStore.Get(meta, fromByte)
 	if err != nil {
-		// Errors are logged in contentStore.Get
+		if IsErrRangeNotSatisfiable(err) {
-		writeStatus(ctx, 404)
+			writeStatus(ctx, http.StatusRequestedRangeNotSatisfiable)
+		} else {
+			// Errors are logged in contentStore.Get
+			writeStatus(ctx, 404)
+		}
 		return
 	}
 	defer content.Close()

modules/matchlist/matchlist.go

@@ -0,0 +1,46 @@
+// Copyright 2019 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 matchlist
+
+import (
+	"strings"
+
+	"github.com/gobwas/glob"
+)
+
+// Matchlist represents a block or allow list
+type Matchlist struct {
+	ruleGlobs []glob.Glob
+}
+
+// NewMatchlist creates a new block or allow list
+func NewMatchlist(rules ...string) (*Matchlist, error) {
+	for i := range rules {
+		rules[i] = strings.ToLower(rules[i])
+	}
+	list := Matchlist{
+		ruleGlobs: make([]glob.Glob, 0, len(rules)),
+	}
+
+	for _, rule := range rules {
+		rg, err := glob.Compile(rule)
+		if err != nil {
+			return nil, err
+		}
+		list.ruleGlobs = append(list.ruleGlobs, rg)
+	}
+
+	return &list, nil
+}
+
+// Match will matches
+func (b *Matchlist) Match(u string) bool {
+	for _, r := range b.ruleGlobs {
+		if r.Match(u) {
+			return true
+		}
+	}
+	return false
+}

modules/migrations/gitea_downloader.go

@@ -14,6 +14,7 @@ import (
 	"strings"
 	"time"
 
+	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/migrations/base"
 	"code.gitea.io/gitea/modules/structs"
@@ -47,7 +48,7 @@ func (f *GiteaDownloaderFactory) New(ctx context.Context, opts base.MigrateOptio
 
 	path := strings.Split(repoNameSpace, "/")
 	if len(path) < 2 {
-		return nil, fmt.Errorf("invalid path")
+		return nil, fmt.Errorf("invalid path: %s", repoNameSpace)
 	}
 
 	repoPath := strings.Join(path[len(path)-2:], "/")
@@ -87,7 +88,7 @@ func NewGiteaDownloader(ctx context.Context, baseURL, repoPath, username, passwo
 		gitea_sdk.SetContext(ctx),
 	)
 	if err != nil {
-		log.Error(fmt.Sprintf("NewGiteaDownloader: %s", err.Error()))
+		log.Error(fmt.Sprintf("Failed to create NewGiteaDownloader for: %s. Error: %v", baseURL, err))
 		return nil, err
 	}
 
@@ -101,12 +102,13 @@ func NewGiteaDownloader(ctx context.Context, baseURL, repoPath, username, passwo
 	// set small maxPerPage since we can only guess
 	// (default would be 50 but this can differ)
 	maxPerPage := 10
-	// new gitea instances can tell us what maximum they have
+	// gitea instances >=1.13 can tell us what maximum they have
-	if giteaClient.CheckServerVersionConstraint(">=1.13.0") == nil {
+	apiConf, _, err := giteaClient.GetGlobalAPISettings()
-		apiConf, _, err := giteaClient.GetGlobalAPISettings()
+	if err != nil {
-		if err != nil {
+		log.Info("Unable to get global API settings. Ignoring these.")
-			return nil, err
+		log.Debug("giteaClient.GetGlobalAPISettings. Error: %v", err)
-		}
+	}
+	if apiConf != nil {
 		maxPerPage = apiConf.MaxResponseItems
 	}
 
@@ -324,45 +326,44 @@ func (g *GiteaDownloader) GetAsset(_ string, relID, id int64) (io.ReadCloser, er
 }
 
 func (g *GiteaDownloader) getIssueReactions(index int64) ([]*base.Reaction, error) {
-	var reactions []*base.Reaction
 	if err := g.client.CheckServerVersionConstraint(">=1.11"); err != nil {
 		log.Info("GiteaDownloader: instance to old, skip getIssueReactions")
-		return reactions, nil
+		return []*base.Reaction{}, nil
 	}
 	rl, _, err := g.client.GetIssueReactions(g.repoOwner, g.repoName, index)
 	if err != nil {
 		return nil, err
 	}
 
-	for _, reaction := range rl {
+	return g.convertReactions(rl), nil
-		reactions = append(reactions, &base.Reaction{
-			UserID:   reaction.User.ID,
-			UserName: reaction.User.UserName,
-			Content:  reaction.Reaction,
-		})
-	}
-	return reactions, nil
 }
 
 func (g *GiteaDownloader) getCommentReactions(commentID int64) ([]*base.Reaction, error) {
-	var reactions []*base.Reaction
 	if err := g.client.CheckServerVersionConstraint(">=1.11"); err != nil {
 		log.Info("GiteaDownloader: instance to old, skip getCommentReactions")
-		return reactions, nil
+		return []*base.Reaction{}, nil
 	}
 	rl, _, err := g.client.GetIssueCommentReactions(g.repoOwner, g.repoName, commentID)
 	if err != nil {
 		return nil, err
 	}
 
+	return g.convertReactions(rl), nil
+}
+
+func (g *GiteaDownloader) convertReactions(rl []*gitea_sdk.Reaction) []*base.Reaction {
+	var reactions []*base.Reaction
 	for i := range rl {
+		if rl[i].User.ID <= 0 {
+			continue
+		}
 		reactions = append(reactions, &base.Reaction{
 			UserID:   rl[i].User.ID,
 			UserName: rl[i].User.UserName,
 			Content:  rl[i].Reaction,
 		})
 	}
-	return reactions, nil
+	return reactions
 }
 
 // GetIssues returns issues according start and limit
@@ -394,7 +395,11 @@ func (g *GiteaDownloader) GetIssues(page, perPage int) ([]*base.Issue, bool, err
 
 		reactions, err := g.getIssueReactions(issue.Index)
 		if err != nil {
-			return nil, false, fmt.Errorf("error while loading reactions: %v", err)
+			log.Warn("Unable to load reactions during migrating issue #%d to %s/%s. Error: %v", issue.Index, g.repoOwner, g.repoName, err)
+			if err2 := models.CreateRepositoryNotice(
+				fmt.Sprintf("Unable to load reactions during migrating issue #%d to %s/%s. Error: %v", issue.Index, g.repoOwner, g.repoName, err)); err2 != nil {
+				log.Error("create repository notice failed: ", err2)
+			}
 		}
 
 		var assignees []string
@@ -445,13 +450,17 @@ func (g *GiteaDownloader) GetComments(index int64) ([]*base.Comment, error) {
 		// Page:     i,
 	}})
 	if err != nil {
-		return nil, fmt.Errorf("error while listing comments: %v", err)
+		return nil, fmt.Errorf("error while listing comments for issue #%d. Error: %v", index, err)
 	}
 
 	for _, comment := range comments {
 		reactions, err := g.getCommentReactions(comment.ID)
 		if err != nil {
-			return nil, fmt.Errorf("error while listing comment creactions: %v", err)
+			log.Warn("Unable to load comment reactions during migrating issue #%d for comment %d to %s/%s. Error: %v", index, comment.ID, g.repoOwner, g.repoName, err)
+			if err2 := models.CreateRepositoryNotice(
+				fmt.Sprintf("Unable to load reactions during migrating issue #%d for comment %d to %s/%s. Error: %v", index, comment.ID, g.repoOwner, g.repoName, err)); err2 != nil {
+				log.Error("create repository notice failed: ", err2)
+			}
 		}
 
 		allComments = append(allComments, &base.Comment{
@@ -489,7 +498,7 @@ func (g *GiteaDownloader) GetPullRequests(page, perPage int) ([]*base.PullReques
 		State: gitea_sdk.StateAll,
 	})
 	if err != nil {
-		return nil, false, fmt.Errorf("error while listing repos: %v", err)
+		return nil, false, fmt.Errorf("error while listing pull requests (page: %d, pagesize: %d). Error: %v", page, perPage, err)
 	}
 	for _, pr := range prs {
 		var milestone string
@@ -520,7 +529,7 @@ func (g *GiteaDownloader) GetPullRequests(page, perPage int) ([]*base.PullReques
 			if headSHA == "" {
 				headCommit, _, err := g.client.GetSingleCommit(g.repoOwner, g.repoName, url.PathEscape(pr.Head.Ref))
 				if err != nil {
-					return nil, false, fmt.Errorf("error while resolving git ref: %v", err)
+					return nil, false, fmt.Errorf("error while resolving head git ref: %s for pull #%d. Error: %v", pr.Head.Ref, pr.Index, err)
 				}
 				headSHA = headCommit.SHA
 			}
@@ -533,7 +542,11 @@ func (g *GiteaDownloader) GetPullRequests(page, perPage int) ([]*base.PullReques
 
 		reactions, err := g.getIssueReactions(pr.Index)
 		if err != nil {
-			return nil, false, fmt.Errorf("error while loading reactions: %v", err)
+			log.Warn("Unable to load reactions during migrating pull #%d to %s/%s. Error: %v", pr.Index, g.repoOwner, g.repoName, err)
+			if err2 := models.CreateRepositoryNotice(
+				fmt.Sprintf("Unable to load reactions during migrating pull #%d to %s/%s. Error: %v", pr.Index, g.repoOwner, g.repoName, err)); err2 != nil {
+				log.Error("create repository notice failed: ", err2)
+			}
 		}
 
 		var assignees []string

modules/migrations/gitea_uploader.go

@@ -28,6 +28,7 @@ import (
 	"code.gitea.io/gitea/modules/storage"
 	"code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/timeutil"
+	"code.gitea.io/gitea/services/pull"
 
 	gouuid "github.com/google/uuid"
 )
@@ -524,6 +525,7 @@ func (g *GiteaLocalUploader) CreatePullRequests(prs ...*base.PullRequest) error
 	}
 	for _, pr := range gprs {
 		g.issues.Store(pr.Issue.Index, pr.Issue.ID)
+		pull.AddToTaskQueue(pr)
 	}
 	return nil
 }

modules/migrations/github.go

@@ -65,23 +65,25 @@ func (f *GithubDownloaderV3Factory) GitServiceType() structs.GitServiceType {
 // GithubDownloaderV3 implements a Downloader interface to get repository informations
 // from github via APIv3
 type GithubDownloaderV3 struct {
-	ctx       context.Context
+	ctx        context.Context
-	client    *github.Client
+	client     *github.Client
-	repoOwner string
+	repoOwner  string
-	repoName  string
+	repoName   string
-	userName  string
+	userName   string
-	password  string
+	password   string
-	rate      *github.Rate
+	rate       *github.Rate
+	maxPerPage int
 }
 
 // NewGithubDownloaderV3 creates a github Downloader via github v3 API
 func NewGithubDownloaderV3(ctx context.Context, baseURL, userName, password, token, repoOwner, repoName string) *GithubDownloaderV3 {
 	var downloader = GithubDownloaderV3{
-		userName:  userName,
+		userName:   userName,
-		password:  password,
+		password:   password,
-		ctx:       ctx,
+		ctx:        ctx,
-		repoOwner: repoOwner,
+		repoOwner:  repoOwner,
-		repoName:  repoName,
+		repoName:   repoName,
+		maxPerPage: 100,
 	}
 
 	client := &http.Client{
@@ -177,7 +179,7 @@ func (g *GithubDownloaderV3) GetTopics() ([]string, error) {
 
 // GetMilestones returns milestones
 func (g *GithubDownloaderV3) GetMilestones() ([]*base.Milestone, error) {
-	var perPage = 100
+	var perPage = g.maxPerPage
 	var milestones = make([]*base.Milestone, 0, perPage)
 	for i := 1; ; i++ {
 		g.sleep()
@@ -233,7 +235,7 @@ func convertGithubLabel(label *github.Label) *base.Label {
 
 // GetLabels returns labels
 func (g *GithubDownloaderV3) GetLabels() ([]*base.Label, error) {
-	var perPage = 100
+	var perPage = g.maxPerPage
 	var labels = make([]*base.Label, 0, perPage)
 	for i := 1; ; i++ {
 		g.sleep()
@@ -304,7 +306,7 @@ func (g *GithubDownloaderV3) convertGithubRelease(rel *github.RepositoryRelease)
 
 // GetReleases returns releases
 func (g *GithubDownloaderV3) GetReleases() ([]*base.Release, error) {
-	var perPage = 100
+	var perPage = g.maxPerPage
 	var releases = make([]*base.Release, 0, perPage)
 	for i := 1; ; i++ {
 		g.sleep()
@@ -342,6 +344,9 @@ func (g *GithubDownloaderV3) GetAsset(_ string, _, id int64) (io.ReadCloser, err
 
 // GetIssues returns issues according start and limit
 func (g *GithubDownloaderV3) GetIssues(page, perPage int) ([]*base.Issue, bool, error) {
+	if perPage > g.maxPerPage {
+		perPage = g.maxPerPage
+	}
 	opt := &github.IssueListByRepoOptions{
 		Sort:      "created",
 		Direction: "asc",
@@ -429,7 +434,7 @@ func (g *GithubDownloaderV3) GetIssues(page, perPage int) ([]*base.Issue, bool,
 // GetComments returns comments according issueNumber
 func (g *GithubDownloaderV3) GetComments(issueNumber int64) ([]*base.Comment, error) {
 	var (
-		allComments = make([]*base.Comment, 0, 100)
+		allComments = make([]*base.Comment, 0, g.maxPerPage)
 		created     = "created"
 		asc         = "asc"
 	)
@@ -437,7 +442,7 @@ func (g *GithubDownloaderV3) GetComments(issueNumber int64) ([]*base.Comment, er
 		Sort:      &created,
 		Direction: &asc,
 		ListOptions: github.ListOptions{
-			PerPage: 100,
+			PerPage: g.maxPerPage,
 		},
 	}
 	for {
@@ -459,7 +464,7 @@ func (g *GithubDownloaderV3) GetComments(issueNumber int64) ([]*base.Comment, er
 				g.sleep()
 				res, resp, err := g.client.Reactions.ListIssueCommentReactions(g.ctx, g.repoOwner, g.repoName, comment.GetID(), &github.ListOptions{
 					Page:    i,
-					PerPage: 100,
+					PerPage: g.maxPerPage,
 				})
 				if err != nil {
 					return nil, err
@@ -497,6 +502,9 @@ func (g *GithubDownloaderV3) GetComments(issueNumber int64) ([]*base.Comment, er
 
 // GetPullRequests returns pull requests according page and perPage
 func (g *GithubDownloaderV3) GetPullRequests(page, perPage int) ([]*base.PullRequest, bool, error) {
+	if perPage > g.maxPerPage {
+		perPage = g.maxPerPage
+	}
 	opt := &github.PullRequestListOptions{
 		Sort:      "created",
 		Direction: "asc",
@@ -650,7 +658,7 @@ func (g *GithubDownloaderV3) convertGithubReviewComments(cs []*github.PullReques
 			g.sleep()
 			res, resp, err := g.client.Reactions.ListPullRequestCommentReactions(g.ctx, g.repoOwner, g.repoName, c.GetID(), &github.ListOptions{
 				Page:    i,
-				PerPage: 100,
+				PerPage: g.maxPerPage,
 			})
 			if err != nil {
 				return nil, err
@@ -687,9 +695,9 @@ func (g *GithubDownloaderV3) convertGithubReviewComments(cs []*github.PullReques
 
 // GetReviews returns pull requests review
 func (g *GithubDownloaderV3) GetReviews(pullRequestNumber int64) ([]*base.Review, error) {
-	var allReviews = make([]*base.Review, 0, 100)
+	var allReviews = make([]*base.Review, 0, g.maxPerPage)
 	opt := &github.ListOptions{
-		PerPage: 100,
+		PerPage: g.maxPerPage,
 	}
 	for {
 		g.sleep()
@@ -703,7 +711,7 @@ func (g *GithubDownloaderV3) GetReviews(pullRequestNumber int64) ([]*base.Review
 			r.IssueIndex = pullRequestNumber
 			// retrieve all review comments
 			opt2 := &github.ListOptions{
-				PerPage: 100,
+				PerPage: g.maxPerPage,
 			}
 			for {
 				g.sleep()

modules/migrations/gitlab.go

@@ -11,6 +11,7 @@ import (
 	"io"
 	"net/http"
 	"net/url"
+	"path"
 	"strings"
 	"time"
 
@@ -68,6 +69,7 @@ type GitlabDownloader struct {
 	repoName        string
 	issueCount      int64
 	fetchPRcomments bool
+	maxPerPage      int
 }
 
 // NewGitlabDownloader creates a gitlab Downloader via gitlab API
@@ -86,6 +88,30 @@ func NewGitlabDownloader(ctx context.Context, baseURL, repoPath, username, passw
 		return nil, err
 	}
 
+	// split namespace and subdirectory
+	pathParts := strings.Split(strings.Trim(repoPath, "/"), "/")
+	var resp *gitlab.Response
+	u, _ := url.Parse(baseURL)
+	for len(pathParts) >= 2 {
+		_, resp, err = gitlabClient.Version.GetVersion()
+		if err == nil || resp != nil && resp.StatusCode == 401 {
+			err = nil // if no authentication given, this still should work
+			break
+		}
+
+		u.Path = path.Join(u.Path, pathParts[0])
+		baseURL = u.String()
+		pathParts = pathParts[1:]
+		_ = gitlab.WithBaseURL(baseURL)(gitlabClient)
+		repoPath = strings.Join(pathParts, "/")
+	}
+	if err != nil {
+		log.Trace("Error could not get gitlab version: %v", err)
+		return nil, err
+	}
+
+	log.Trace("gitlab downloader: use BaseURL: '%s' and RepoPath: '%s'", baseURL, repoPath)
+
 	// Grab and store project/repo ID here, due to issues using the URL escaped path
 	gr, _, err := gitlabClient.Projects.GetProject(repoPath, nil, nil, gitlab.WithContext(ctx))
 	if err != nil {
@@ -99,10 +125,11 @@ func NewGitlabDownloader(ctx context.Context, baseURL, repoPath, username, passw
 	}
 
 	return &GitlabDownloader{
-		ctx:      ctx,
+		ctx:        ctx,
-		client:   gitlabClient,
+		client:     gitlabClient,
-		repoID:   gr.ID,
+		repoID:     gr.ID,
-		repoName: gr.Name,
+		repoName:   gr.Name,
+		maxPerPage: 100,
 	}, nil
 }
 
@@ -159,7 +186,7 @@ func (g *GitlabDownloader) GetTopics() ([]string, error) {
 
 // GetMilestones returns milestones
 func (g *GitlabDownloader) GetMilestones() ([]*base.Milestone, error) {
-	var perPage = 100
+	var perPage = g.maxPerPage
 	var state = "all"
 	var milestones = make([]*base.Milestone, 0, perPage)
 	for i := 1; ; i++ {
@@ -230,7 +257,7 @@ func (g *GitlabDownloader) normalizeColor(val string) string {
 
 // GetLabels returns labels
 func (g *GitlabDownloader) GetLabels() ([]*base.Label, error) {
-	var perPage = 100
+	var perPage = g.maxPerPage
 	var labels = make([]*base.Label, 0, perPage)
 	for i := 1; ; i++ {
 		ls, _, err := g.client.Labels.ListLabels(g.repoID, &gitlab.ListLabelsOptions{ListOptions: gitlab.ListOptions{
@@ -281,7 +308,7 @@ func (g *GitlabDownloader) convertGitlabRelease(rel *gitlab.Release) *base.Relea
 
 // GetReleases returns releases
 func (g *GitlabDownloader) GetReleases() ([]*base.Release, error) {
-	var perPage = 100
+	var perPage = g.maxPerPage
 	var releases = make([]*base.Release, 0, perPage)
 	for i := 1; ; i++ {
 		ls, _, err := g.client.Releases.ListReleases(g.repoID, &gitlab.ListReleasesOptions{
@@ -330,6 +357,10 @@ func (g *GitlabDownloader) GetIssues(page, perPage int) ([]*base.Issue, bool, er
 	state := "all"
 	sort := "asc"
 
+	if perPage > g.maxPerPage {
+		perPage = g.maxPerPage
+	}
+
 	opt := &gitlab.ListProjectIssuesOptions{
 		State: &state,
 		Sort:  &sort,
@@ -401,7 +432,7 @@ func (g *GitlabDownloader) GetIssues(page, perPage int) ([]*base.Issue, bool, er
 // GetComments returns comments according issueNumber
 // TODO: figure out how to transfer comment reactions
 func (g *GitlabDownloader) GetComments(issueNumber int64) ([]*base.Comment, error) {
-	var allComments = make([]*base.Comment, 0, 100)
+	var allComments = make([]*base.Comment, 0, g.maxPerPage)
 
 	var page = 1
 	var realIssueNumber int64
@@ -415,14 +446,14 @@ func (g *GitlabDownloader) GetComments(issueNumber int64) ([]*base.Comment, erro
 			realIssueNumber = issueNumber
 			comments, resp, err = g.client.Discussions.ListIssueDiscussions(g.repoID, int(realIssueNumber), &gitlab.ListIssueDiscussionsOptions{
 				Page:    page,
-				PerPage: 100,
+				PerPage: g.maxPerPage,
 			}, nil, gitlab.WithContext(g.ctx))
 		} else {
 			// If this is a PR, we need to figure out the Gitlab/original PR ID to be passed below
 			realIssueNumber = issueNumber - g.issueCount
 			comments, resp, err = g.client.Discussions.ListMergeRequestDiscussions(g.repoID, int(realIssueNumber), &gitlab.ListMergeRequestDiscussionsOptions{
 				Page:    page,
-				PerPage: 100,
+				PerPage: g.maxPerPage,
 			}, nil, gitlab.WithContext(g.ctx))
 		}
 
@@ -465,6 +496,10 @@ func (g *GitlabDownloader) GetComments(issueNumber int64) ([]*base.Comment, erro
 
 // GetPullRequests returns pull requests according page and perPage
 func (g *GitlabDownloader) GetPullRequests(page, perPage int) ([]*base.PullRequest, bool, error) {
+	if perPage > g.maxPerPage {
+		perPage = g.maxPerPage
+	}
+
 	opt := &gitlab.ListProjectMergeRequestsOptions{
 		ListOptions: gitlab.ListOptions{
 			PerPage: perPage,
@@ -574,8 +609,12 @@ func (g *GitlabDownloader) GetPullRequests(page, perPage int) ([]*base.PullReque
 
 // GetReviews returns pull requests review
 func (g *GitlabDownloader) GetReviews(pullRequestNumber int64) ([]*base.Review, error) {
-	state, _, err := g.client.MergeRequestApprovals.GetApprovalState(g.repoID, int(pullRequestNumber), gitlab.WithContext(g.ctx))
+	state, resp, err := g.client.MergeRequestApprovals.GetApprovalState(g.repoID, int(pullRequestNumber), gitlab.WithContext(g.ctx))
 	if err != nil {
+		if resp != nil && resp.StatusCode == 404 {
+			log.Error(fmt.Sprintf("GitlabDownloader: while migrating a error occurred: '%s'", err.Error()))
+			return []*base.Review{}, nil
+		}
 		return nil, err
 	}
 

modules/migrations/migrate.go

@@ -8,9 +8,13 @@ package migrations
 import (
 	"context"
 	"fmt"
+	"net"
+	"net/url"
+	"strings"
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/matchlist"
 	"code.gitea.io/gitea/modules/migrations/base"
 	"code.gitea.io/gitea/modules/setting"
 )
@@ -20,6 +24,9 @@ type MigrateOptions = base.MigrateOptions
 
 var (
 	factories []base.DownloaderFactory
+
+	allowList *matchlist.Matchlist
+	blockList *matchlist.Matchlist
 )
 
 // RegisterDownloaderFactory registers a downloader factory
@@ -27,12 +34,49 @@ func RegisterDownloaderFactory(factory base.DownloaderFactory) {
 	factories = append(factories, factory)
 }
 
+func isMigrateURLAllowed(remoteURL string) error {
+	u, err := url.Parse(strings.ToLower(remoteURL))
+	if err != nil {
+		return err
+	}
+
+	if strings.EqualFold(u.Scheme, "http") || strings.EqualFold(u.Scheme, "https") {
+		if len(setting.Migrations.AllowedDomains) > 0 {
+			if !allowList.Match(u.Host) {
+				return &models.ErrMigrationNotAllowed{Host: u.Host}
+			}
+		} else {
+			if blockList.Match(u.Host) {
+				return &models.ErrMigrationNotAllowed{Host: u.Host}
+			}
+		}
+	}
+
+	if !setting.Migrations.AllowLocalNetworks {
+		addrList, err := net.LookupIP(strings.Split(u.Host, ":")[0])
+		if err != nil {
+			return &models.ErrMigrationNotAllowed{Host: u.Host, NotResolvedIP: true}
+		}
+		for _, addr := range addrList {
+			if isIPPrivate(addr) || !addr.IsGlobalUnicast() {
+				return &models.ErrMigrationNotAllowed{Host: u.Host, PrivateNet: addr.String()}
+			}
+		}
+	}
+
+	return nil
+}
+
 // MigrateRepository migrate repository according MigrateOptions
 func MigrateRepository(ctx context.Context, doer *models.User, ownerName string, opts base.MigrateOptions) (*models.Repository, error) {
+	err := isMigrateURLAllowed(opts.CloneAddr)
+	if err != nil {
+		return nil, err
+	}
+
 	var (
 		downloader base.Downloader
 		uploader   = NewGiteaLocalUploader(ctx, doer, ownerName, opts.RepoName)
-		err        error
 	)
 
 	for _, factory := range factories {
@@ -69,7 +113,7 @@ func MigrateRepository(ctx context.Context, doer *models.User, ownerName string,
 		}
 
 		if err2 := models.CreateRepositoryNotice(fmt.Sprintf("Migrate repository from %s failed: %v", opts.OriginalURL, err)); err2 != nil {
-			log.Error("create respotiry notice failed: ", err2)
+			log.Error("create repository notice failed: ", err2)
 		}
 		return nil, err
 	}
@@ -308,3 +352,32 @@ func migrateRepository(downloader base.Downloader, uploader base.Uploader, opts
 
 	return nil
 }
+
+// Init migrations service
+func Init() error {
+	var err error
+	allowList, err = matchlist.NewMatchlist(setting.Migrations.AllowedDomains...)
+	if err != nil {
+		return fmt.Errorf("init migration allowList domains failed: %v", err)
+	}
+
+	blockList, err = matchlist.NewMatchlist(setting.Migrations.BlockedDomains...)
+	if err != nil {
+		return fmt.Errorf("init migration blockList domains failed: %v", err)
+	}
+
+	return nil
+}
+
+// isIPPrivate reports whether ip is a private address, according to
+// RFC 1918 (IPv4 addresses) and RFC 4193 (IPv6 addresses).
+// from https://github.com/golang/go/pull/42793
+// TODO remove if https://github.com/golang/go/issues/29146 got resolved
+func isIPPrivate(ip net.IP) bool {
+	if ip4 := ip.To4(); ip4 != nil {
+		return ip4[0] == 10 ||
+			(ip4[0] == 172 && ip4[1]&0xf0 == 16) ||
+			(ip4[0] == 192 && ip4[1] == 168)
+	}
+	return len(ip) == net.IPv6len && ip[0]&0xfe == 0xfc
+}

modules/migrations/migrate_test.go

@@ -0,0 +1,34 @@
+// Copyright 2019 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 migrations
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/modules/setting"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMigrateWhiteBlocklist(t *testing.T) {
+	setting.Migrations.AllowedDomains = []string{"github.com"}
+	assert.NoError(t, Init())
+
+	err := isMigrateURLAllowed("https://gitlab.com/gitlab/gitlab.git")
+	assert.Error(t, err)
+
+	err = isMigrateURLAllowed("https://github.com/go-gitea/gitea.git")
+	assert.NoError(t, err)
+
+	setting.Migrations.AllowedDomains = []string{}
+	setting.Migrations.BlockedDomains = []string{"github.com"}
+	assert.NoError(t, Init())
+
+	err = isMigrateURLAllowed("https://gitlab.com/gitlab/gitlab.git")
+	assert.NoError(t, err)
+
+	err = isMigrateURLAllowed("https://github.com/go-gitea/gitea.git")
+	assert.Error(t, err)
+}

modules/notification/action/action.go

@@ -314,7 +314,7 @@ func (a *actionNotifier) NotifySyncDeleteRef(doer *models.User, repo *models.Rep
 	if err := models.NotifyWatchers(&models.Action{
 		ActUserID: repo.OwnerID,
 		ActUser:   repo.MustOwner(),
-		OpType:    models.ActionMirrorSyncCreate,
+		OpType:    models.ActionMirrorSyncDelete,
 		RepoID:    repo.ID,
 		Repo:      repo,
 		IsPrivate: repo.IsPrivate,

modules/notification/webhook/webhook.go

@@ -797,3 +797,11 @@ func (m *webhookNotifier) NotifySyncPushCommits(pusher *models.User, repo *model
 		log.Error("PrepareWebhooks: %v", err)
 	}
 }
+
+func (m *webhookNotifier) NotifySyncCreateRef(pusher *models.User, repo *models.Repository, refType, refFullName string) {
+	m.NotifyCreateRef(pusher, repo, refType, refFullName)
+}
+
+func (m *webhookNotifier) NotifySyncDeleteRef(pusher *models.User, repo *models.Repository, refType, refFullName string) {
+	m.NotifyDeleteRef(pusher, repo, refType, refFullName)
+}

modules/references/references.go

@@ -235,40 +235,78 @@ func findAllIssueReferencesMarkdown(content string) []*rawReference {
 	return findAllIssueReferencesBytes(bcontent, links)
 }
 
+func convertFullHTMLReferencesToShortRefs(re *regexp.Regexp, contentBytes *[]byte) {
+	// We will iterate through the content, rewrite and simplify full references.
+	//
+	// We want to transform something like:
+	//
+	// this is a https://ourgitea.com/git/owner/repo/issues/123456789, foo
+	// https://ourgitea.com/git/owner/repo/pulls/123456789
+	//
+	// Into something like:
+	//
+	// this is a #123456789, foo
+	// !123456789
+
+	pos := 0
+	for {
+		// re looks for something like: (\s|^|\(|\[)https://ourgitea.com/git/(owner/repo)/(issues)/(123456789)(?:\s|$|\)|\]|[:;,.?!]\s|[:;,.?!]$)
+		match := re.FindSubmatchIndex((*contentBytes)[pos:])
+		if match == nil {
+			break
+		}
+		// match is a bunch of indices into the content from pos onwards so
+		// to simplify things let's just add pos to all of the indices in match
+		for i := range match {
+			match[i] += pos
+		}
+
+		// match[0]-match[1] is whole string
+		// match[2]-match[3] is preamble
+
+		// move the position to the end of the preamble
+		pos = match[3]
+
+		// match[4]-match[5] is owner/repo
+		// now copy the owner/repo to end of the preamble
+		endPos := pos + match[5] - match[4]
+		copy((*contentBytes)[pos:endPos], (*contentBytes)[match[4]:match[5]])
+
+		// move the current position to the end of the newly copied owner/repo
+		pos = endPos
+
+		// Now set the issue/pull marker:
+		//
+		// match[6]-match[7] == 'issues'
+		(*contentBytes)[pos] = '#'
+		if string((*contentBytes)[match[6]:match[7]]) == "pulls" {
+			(*contentBytes)[pos] = '!'
+		}
+		pos++
+
+		// Then add the issue/pull number
+		//
+		// match[8]-match[9] is the number
+		endPos = pos + match[9] - match[8]
+		copy((*contentBytes)[pos:endPos], (*contentBytes)[match[8]:match[9]])
+
+		// Now copy what's left at the end of the string to the new end position
+		copy((*contentBytes)[endPos:], (*contentBytes)[match[9]:])
+		// now we reset the length
+
+		// our new section has length endPos - match[3]
+		// our old section has length match[9] - match[3]
+		(*contentBytes) = (*contentBytes)[:len((*contentBytes))-match[9]+endPos]
+		pos = endPos
+	}
+}
+
 // FindAllIssueReferences returns a list of unvalidated references found in a string.
 func FindAllIssueReferences(content string) []IssueReference {
 	// Need to convert fully qualified html references to local system to #/! short codes
 	contentBytes := []byte(content)
 	if re := getGiteaIssuePullPattern(); re != nil {
-		pos := 0
+		convertFullHTMLReferencesToShortRefs(re, &contentBytes)
-		for {
-			match := re.FindSubmatchIndex(contentBytes[pos:])
-			if match == nil {
-				break
-			}
-			// match[0]-match[1] is whole string
-			// match[2]-match[3] is preamble
-			pos += match[3]
-			// match[4]-match[5] is owner/repo
-			endPos := pos + match[5] - match[4]
-			copy(contentBytes[pos:endPos], contentBytes[match[4]:match[5]])
-			pos = endPos
-			// match[6]-match[7] == 'issues'
-			contentBytes[pos] = '#'
-			if string(contentBytes[match[6]:match[7]]) == "pulls" {
-				contentBytes[pos] = '!'
-			}
-			pos++
-			// match[8]-match[9] is the number
-			endPos = pos + match[9] - match[8]
-			copy(contentBytes[pos:endPos], contentBytes[match[8]:match[9]])
-			copy(contentBytes[endPos:], contentBytes[match[9]:])
-			// now we reset the length
-			// our new section has length endPos - match[3]
-			// our old section has length match[9] - match[3]
-			contentBytes = contentBytes[:len(contentBytes)-match[9]+endPos]
-			pos = endPos
-		}
 	} else {
 		log.Debug("No GiteaIssuePullPattern pattern")
 	}

modules/references/references_test.go

@@ -5,6 +5,7 @@
 package references
 
 import (
+	"regexp"
 	"testing"
 
 	"code.gitea.io/gitea/modules/setting"
@@ -29,6 +30,26 @@ type testResult struct {
 	TimeLog        string
 }
 
+func TestConvertFullHTMLReferencesToShortRefs(t *testing.T) {
+	re := regexp.MustCompile(`(\s|^|\(|\[)` +
+		regexp.QuoteMeta("https://ourgitea.com/git/") +
+		`([0-9a-zA-Z-_\.]+/[0-9a-zA-Z-_\.]+)/` +
+		`((?:issues)|(?:pulls))/([0-9]+)(?:\s|$|\)|\]|[:;,.?!]\s|[:;,.?!]$)`)
+	test := `this is a https://ourgitea.com/git/owner/repo/issues/123456789, foo
+https://ourgitea.com/git/owner/repo/pulls/123456789
+  And https://ourgitea.com/git/owner/repo/pulls/123
+`
+	expect := `this is a owner/repo#123456789, foo
+owner/repo!123456789
+  And owner/repo!123
+`
+
+	contentBytes := []byte(test)
+	convertFullHTMLReferencesToShortRefs(re, &contentBytes)
+	result := string(contentBytes)
+	assert.EqualValues(t, expect, result)
+}
+
 func TestFindAllIssueReferences(t *testing.T) {
 
 	fixtures := []testFixture{
@@ -106,6 +127,13 @@ func TestFindAllIssueReferences(t *testing.T) {
 				{202, "user4", "repo5", "202", true, XRefActionNone, nil, nil, ""},
 			},
 		},
+		{
+			"This http://gitea.com:3000/user4/repo5/pulls/202 yes. http://gitea.com:3000/user4/repo5/pulls/203 no",
+			[]testResult{
+				{202, "user4", "repo5", "202", true, XRefActionNone, nil, nil, ""},
+				{203, "user4", "repo5", "203", true, XRefActionNone, nil, nil, ""},
+			},
+		},
 		{
 			"This http://GiTeA.COM:3000/user4/repo6/pulls/205 yes.",
 			[]testResult{

modules/repository/init.go

@@ -162,10 +162,10 @@ func initRepoCommit(tmpPath string, repo *models.Repository, u *models.User, def
 		defaultBranch = setting.Repository.DefaultBranch
 	}
 
-	if stdout, err := git.NewCommand("push", "origin", "master:"+defaultBranch).
+	if stdout, err := git.NewCommand("push", "origin", "HEAD:"+defaultBranch).
 		SetDescription(fmt.Sprintf("initRepoCommit (git push): %s", tmpPath)).
 		RunInDirWithEnv(tmpPath, models.InternalPushingEnvironment(u, repo)); err != nil {
-		log.Error("Failed to push back to master: Stdout: %s\nError: %v", stdout, err)
+		log.Error("Failed to push back to HEAD: Stdout: %s\nError: %v", stdout, err)
 		return fmt.Errorf("git push: %v", err)
 	}
 

modules/setting/migrations.go

@@ -4,11 +4,18 @@
 
 package setting
 
+import (
+	"strings"
+)
+
 var (
 	// Migrations settings
 	Migrations = struct {
-		MaxAttempts  int
+		MaxAttempts        int
-		RetryBackoff int
+		RetryBackoff       int
+		AllowedDomains     []string
+		BlockedDomains     []string
+		AllowLocalNetworks bool
 	}{
 		MaxAttempts:  3,
 		RetryBackoff: 3,
@@ -19,4 +26,15 @@ func newMigrationsService() {
 	sec := Cfg.Section("migrations")
 	Migrations.MaxAttempts = sec.Key("MAX_ATTEMPTS").MustInt(Migrations.MaxAttempts)
 	Migrations.RetryBackoff = sec.Key("RETRY_BACKOFF").MustInt(Migrations.RetryBackoff)
+
+	Migrations.AllowedDomains = sec.Key("ALLOWED_DOMAINS").Strings(",")
+	for i := range Migrations.AllowedDomains {
+		Migrations.AllowedDomains[i] = strings.ToLower(Migrations.AllowedDomains[i])
+	}
+	Migrations.BlockedDomains = sec.Key("BLOCKED_DOMAINS").Strings(",")
+	for i := range Migrations.BlockedDomains {
+		Migrations.BlockedDomains[i] = strings.ToLower(Migrations.BlockedDomains[i])
+	}
+
+	Migrations.AllowLocalNetworks = sec.Key("ALLOW_LOCALNETWORKS").MustBool(false)
 }

modules/setting/repository.go

@@ -143,7 +143,7 @@ var (
 		MaxCreationLimit:                        -1,
 		MirrorQueueLength:                       1000,
 		PullRequestQueueLength:                  1000,
-		PreferredLicenses:                       []string{"Apache License 2.0,MIT License"},
+		PreferredLicenses:                       []string{"Apache License 2.0", "MIT License"},
 		DisableHTTPGit:                          false,
 		AccessControlAllowOrigin:                "",
 		UseCompatSSHURI:                         false,

modules/setting/storage.go

@@ -21,7 +21,7 @@ type Storage struct {
 
 // MapTo implements the Mappable interface
 func (s *Storage) MapTo(v interface{}) error {
-	pathValue := reflect.ValueOf(v).FieldByName("Path")
+	pathValue := reflect.ValueOf(v).Elem().FieldByName("Path")
 	if pathValue.IsValid() && pathValue.Kind() == reflect.String {
 		pathValue.SetString(s.Path)
 	}
@@ -32,21 +32,19 @@ func (s *Storage) MapTo(v interface{}) error {
 }
 
 func getStorage(name, typ string, overrides ...*ini.Section) Storage {
-	sectionName := "storage"
+	const sectionName = "storage"
-	if len(name) > 0 {
-		sectionName = sectionName + "." + typ
-	}
 	sec := Cfg.Section(sectionName)
 
 	if len(overrides) == 0 {
 		overrides = []*ini.Section{
+			Cfg.Section(sectionName + "." + typ),
 			Cfg.Section(sectionName + "." + name),
 		}
 	}
 
 	var storage Storage
 
-	storage.Type = sec.Key("STORAGE_TYPE").MustString("")
+	storage.Type = sec.Key("STORAGE_TYPE").MustString(typ)
 	storage.ServeDirect = sec.Key("SERVE_DIRECT").MustBool(false)
 
 	// Global Defaults

modules/storage/local.go

@@ -11,6 +11,7 @@ import (
 	"os"
 	"path/filepath"
 
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/util"
 )
 
@@ -39,7 +40,7 @@ func NewLocalStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error
 		return nil, err
 	}
 	config := configInterface.(LocalStorageConfig)
-
+	log.Info("Creating new Local Storage at %s", config.Path)
 	if err := os.MkdirAll(config.Path, os.ModePerm); err != nil {
 		return nil, err
 	}

modules/storage/minio.go

@@ -13,6 +13,7 @@ import (
 	"strings"
 	"time"
 
+	"code.gitea.io/gitea/modules/log"
 	"github.com/minio/minio-go/v7"
 	"github.com/minio/minio-go/v7/pkg/credentials"
 )
@@ -30,7 +31,7 @@ type minioObject struct {
 func (m *minioObject) Stat() (os.FileInfo, error) {
 	oi, err := m.Object.Stat()
 	if err != nil {
-		return nil, err
+		return nil, convertMinioErr(err)
 	}
 
 	return &minioFileInfo{oi}, nil
@@ -58,20 +59,41 @@ type MinioStorage struct {
 	basePath string
 }
 
+func convertMinioErr(err error) error {
+	if err == nil {
+		return nil
+	}
+	errResp, ok := err.(minio.ErrorResponse)
+	if !ok {
+		return err
+	}
+
+	// Convert two responses to standard analogues
+	switch errResp.Code {
+	case "NoSuchKey":
+		return os.ErrNotExist
+	case "AccessDenied":
+		return os.ErrPermission
+	}
+
+	return err
+}
+
 // NewMinioStorage returns a minio storage
 func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error) {
 	configInterface, err := toConfig(MinioStorageConfig{}, cfg)
 	if err != nil {
-		return nil, err
+		return nil, convertMinioErr(err)
 	}
 	config := configInterface.(MinioStorageConfig)
 
+	log.Info("Creating Minio storage at %s:%s with base path %s", config.Endpoint, config.Bucket, config.BasePath)
 	minioClient, err := minio.New(config.Endpoint, &minio.Options{
 		Creds:  credentials.NewStaticV4(config.AccessKeyID, config.SecretAccessKey, ""),
 		Secure: config.UseSSL,
 	})
 	if err != nil {
-		return nil, err
+		return nil, convertMinioErr(err)
 	}
 
 	if err := minioClient.MakeBucket(ctx, config.Bucket, minio.MakeBucketOptions{
@@ -80,7 +102,7 @@ func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error
 		// Check to see if we already own this bucket (which happens if you run this twice)
 		exists, errBucketExists := minioClient.BucketExists(ctx, config.Bucket)
 		if !exists || errBucketExists != nil {
-			return nil, err
+			return nil, convertMinioErr(err)
 		}
 	}
 
@@ -101,7 +123,7 @@ func (m *MinioStorage) Open(path string) (Object, error) {
 	var opts = minio.GetObjectOptions{}
 	object, err := m.client.GetObject(m.ctx, m.bucket, m.buildMinioPath(path), opts)
 	if err != nil {
-		return nil, err
+		return nil, convertMinioErr(err)
 	}
 	return &minioObject{object}, nil
 }
@@ -117,7 +139,7 @@ func (m *MinioStorage) Save(path string, r io.Reader) (int64, error) {
 		minio.PutObjectOptions{ContentType: "application/octet-stream"},
 	)
 	if err != nil {
-		return 0, err
+		return 0, convertMinioErr(err)
 	}
 	return uploadInfo.Size, nil
 }
@@ -164,14 +186,17 @@ func (m *MinioStorage) Stat(path string) (os.FileInfo, error) {
 				return nil, os.ErrNotExist
 			}
 		}
-		return nil, err
+		return nil, convertMinioErr(err)
 	}
 	return &minioFileInfo{info}, nil
 }
 
 // Delete delete a file
 func (m *MinioStorage) Delete(path string) error {
-	return m.client.RemoveObject(m.ctx, m.bucket, m.buildMinioPath(path), minio.RemoveObjectOptions{})
+	if err := m.client.RemoveObject(m.ctx, m.bucket, m.buildMinioPath(path), minio.RemoveObjectOptions{}); err != nil {
+		return convertMinioErr(err)
+	}
+	return nil
 }
 
 // URL gets the redirect URL to a file. The presigned link is valid for 5 minutes.
@@ -179,7 +204,8 @@ func (m *MinioStorage) URL(path, name string) (*url.URL, error) {
 	reqParams := make(url.Values)
 	// TODO it may be good to embed images with 'inline' like ServeData does, but we don't want to have to read the file, do we?
 	reqParams.Set("response-content-disposition", "attachment; filename=\""+quoteEscaper.Replace(name)+"\"")
-	return m.client.PresignedGetObject(m.ctx, m.bucket, m.buildMinioPath(path), 5*time.Minute, reqParams)
+	u, err := m.client.PresignedGetObject(m.ctx, m.bucket, m.buildMinioPath(path), 5*time.Minute, reqParams)
+	return u, convertMinioErr(err)
 }
 
 // IterateObjects iterates across the objects in the miniostorage
@@ -193,13 +219,13 @@ func (m *MinioStorage) IterateObjects(fn func(path string, obj Object) error) er
 	}) {
 		object, err := m.client.GetObject(lobjectCtx, m.bucket, mObjInfo.Key, opts)
 		if err != nil {
-			return err
+			return convertMinioErr(err)
 		}
 		if err := func(object *minio.Object, fn func(path string, obj Object) error) error {
 			defer object.Close()
 			return fn(strings.TrimPrefix(m.basePath, mObjInfo.Key), &minioObject{object})
 		}(object, fn); err != nil {
-			return err
+			return convertMinioErr(err)
 		}
 	}
 	return nil

modules/storage/storage.go

@@ -12,6 +12,7 @@ import (
 	"net/url"
 	"os"
 
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 )
 
@@ -141,21 +142,25 @@ func NewStorage(typStr string, cfg interface{}) (ObjectStorage, error) {
 }
 
 func initAvatars() (err error) {
-	Avatars, err = NewStorage(setting.Avatar.Storage.Type, setting.Avatar.Storage)
+	log.Info("Initialising Avatar storage with type: %s", setting.Avatar.Storage.Type)
+	Avatars, err = NewStorage(setting.Avatar.Storage.Type, &setting.Avatar.Storage)
 	return
 }
 
 func initAttachments() (err error) {
-	Attachments, err = NewStorage(setting.Attachment.Storage.Type, setting.Attachment.Storage)
+	log.Info("Initialising Attachment storage with type: %s", setting.Attachment.Storage.Type)
+	Attachments, err = NewStorage(setting.Attachment.Storage.Type, &setting.Attachment.Storage)
 	return
 }
 
 func initLFS() (err error) {
-	LFS, err = NewStorage(setting.LFS.Storage.Type, setting.LFS.Storage)
+	log.Info("Initialising LFS storage with type: %s", setting.LFS.Storage.Type)
+	LFS, err = NewStorage(setting.LFS.Storage.Type, &setting.LFS.Storage)
 	return
 }
 
 func initRepoAvatars() (err error) {
-	RepoAvatars, err = NewStorage(setting.RepoAvatar.Storage.Type, setting.RepoAvatar.Storage)
+	log.Info("Initialising Repository Avatar storage with type: %s", setting.RepoAvatar.Storage.Type)
+	RepoAvatars, err = NewStorage(setting.RepoAvatar.Storage.Type, &setting.RepoAvatar.Storage)
 	return
 }

modules/task/migrate.go

@@ -20,7 +20,7 @@ import (
 	"code.gitea.io/gitea/modules/util"
 )
 
-func handleCreateError(owner *models.User, err error, name string) error {
+func handleCreateError(owner *models.User, err error) error {
 	switch {
 	case models.IsErrReachLimitOfRepo(err):
 		return fmt.Errorf("You have already reached your limit of %d repositories", owner.MaxCreationLimit())
@@ -38,8 +38,8 @@ func handleCreateError(owner *models.User, err error, name string) error {
 func runMigrateTask(t *models.Task) (err error) {
 	defer func() {
 		if e := recover(); e != nil {
-			err = fmt.Errorf("PANIC whilst trying to do migrate task: %v\nStacktrace: %v", err, log.Stack(2))
+			err = fmt.Errorf("PANIC whilst trying to do migrate task: %v", e)
-			log.Critical("PANIC during runMigrateTask[%d] by DoerID[%d] to RepoID[%d] for OwnerID[%d]: %v", t.ID, t.DoerID, t.RepoID, t.OwnerID, err)
+			log.Critical("PANIC during runMigrateTask[%d] by DoerID[%d] to RepoID[%d] for OwnerID[%d]: %v\nStacktrace: %v", t.ID, t.DoerID, t.RepoID, t.OwnerID, e, log.Stack(2))
 		}
 
 		if err == nil {
@@ -55,7 +55,8 @@ func runMigrateTask(t *models.Task) (err error) {
 		t.EndTime = timeutil.TimeStampNow()
 		t.Status = structs.TaskStatusFailed
 		t.Errors = err.Error()
-		if err := t.UpdateCols("status", "errors", "end_time"); err != nil {
+		t.RepoID = 0
+		if err := t.UpdateCols("status", "errors", "repo_id", "end_time"); err != nil {
 			log.Error("Task UpdateCols failed: %v", err)
 		}
 
@@ -66,8 +67,8 @@ func runMigrateTask(t *models.Task) (err error) {
 		}
 	}()
 
-	if err := t.LoadRepo(); err != nil {
+	if err = t.LoadRepo(); err != nil {
-		return err
+		return
 	}
 
 	// if repository is ready, then just finsih the task
@@ -75,33 +76,35 @@ func runMigrateTask(t *models.Task) (err error) {
 		return nil
 	}
 
-	if err := t.LoadDoer(); err != nil {
+	if err = t.LoadDoer(); err != nil {
-		return err
+		return
 	}
-	if err := t.LoadOwner(); err != nil {
+	if err = t.LoadOwner(); err != nil {
-		return err
+		return
 	}
 	t.StartTime = timeutil.TimeStampNow()
 	t.Status = structs.TaskStatusRunning
-	if err := t.UpdateCols("start_time", "status"); err != nil {
+	if err = t.UpdateCols("start_time", "status"); err != nil {
-		return err
+		return
 	}
 
 	var opts *migration.MigrateOptions
 	opts, err = t.MigrateConfig()
 	if err != nil {
-		return err
+		return
 	}
 
 	opts.MigrateToRepoID = t.RepoID
-	repo, err := migrations.MigrateRepository(graceful.GetManager().HammerContext(), t.Doer, t.Owner.Name, *opts)
+	var repo *models.Repository
+	repo, err = migrations.MigrateRepository(graceful.GetManager().HammerContext(), t.Doer, t.Owner.Name, *opts)
 	if err == nil {
 		log.Trace("Repository migrated [%d]: %s/%s", repo.ID, t.Owner.Name, repo.Name)
-		return nil
+		return
 	}
 
 	if models.IsErrRepoAlreadyExist(err) {
-		return errors.New("The repository name is already used")
+		err = errors.New("The repository name is already used")
+		return
 	}
 
 	// remoteAddr may contain credentials, so we sanitize it
@@ -113,5 +116,7 @@ func runMigrateTask(t *models.Task) (err error) {
 		return fmt.Errorf("Migration failed: %v", err.Error())
 	}
 
-	return handleCreateError(t.Owner, err, "MigratePost")
+	// do not be tempted to coalesce this line with the return
+	err = handleCreateError(t.Owner, err)
+	return
 }

options/locale/locale_en-US.ini

@@ -366,6 +366,7 @@ org_name_been_taken = The organization name is already taken.
 team_name_been_taken = The team name is already taken.
 team_no_units_error = Allow access to at least one repository section.
 email_been_used = The email address is already used.
+email_invalid = The email address is invalid.
 openid_been_used = The OpenID address '%s' is already used.
 username_password_incorrect = Username or password is incorrect.
 password_complexity = Password does not pass complexity requirements:
@@ -870,9 +871,11 @@ editor.file_already_exists = A file named '%s' already exists in this repository
 editor.commit_empty_file_header = Commit an empty file
 editor.commit_empty_file_text = The file you're about to commit is empty. Proceed?
 editor.no_changes_to_show = There are no changes to show.
-editor.fail_to_update_file = Failed to update/create file '%s' with error: %v
+editor.fail_to_update_file = Failed to update/create file '%s'.
+editor.fail_to_update_file_summary = Error Message:
 editor.push_rejected_no_message = The change was rejected by the server without a message. Please check githooks.
-editor.push_rejected = The change was rejected by the server with the following message:<br>%s<br> Please check githooks.
+editor.push_rejected = The change was rejected by the server. Please check githooks.
+editor.push_rejected_summary = Full Rejection Message:
 editor.add_subdir = Add a directory…
 editor.unable_to_upload_files = Failed to upload files to '%s' with error: %v
 editor.upload_file_is_locked = File '%s' is locked by %s.
@@ -1190,6 +1193,7 @@ issues.review.remove_review_request_self = "refused to review %s"
 issues.review.pending = Pending
 issues.review.review = Review
 issues.review.reviewers = Reviewers
+issues.review.outdated = Outdated
 issues.review.show_outdated = Show outdated
 issues.review.hide_outdated = Hide outdated
 issues.review.show_resolved = Show resolved
@@ -1258,11 +1262,15 @@ pulls.rebase_merge_commit_pull_request = Rebase and Merge (--no-ff)
 pulls.squash_merge_pull_request = Squash and Merge
 pulls.require_signed_wont_sign = The branch requires signed commits but this merge will not be signed
 pulls.invalid_merge_option = You cannot use this merge option for this pull request.
-pulls.merge_conflict = Merge Failed: There was a conflict whilst merging: %[1]s<br>%[2]s<br>Hint: Try a different strategy
+pulls.merge_conflict = Merge Failed: There was a conflict whilst merging. Hint: Try a different strategy
-pulls.rebase_conflict = Merge Failed: There was a conflict whilst rebasing commit: %[1]s<br>%[2]s<br>%[3]s<br>Hint:Try a different strategy
+pulls.merge_conflict_summary = Error Message
+pulls.rebase_conflict = Merge Failed: There was a conflict whilst rebasing commit: %[1]s. Hint: Try a different strategy
+pulls.rebase_conflict_summary = Error Message
+; </summary><code>%[2]s<br>%[3]s</code></details>
 pulls.unrelated_histories = Merge Failed: The merge head and base do not share a common history. Hint: Try a different strategy
 pulls.merge_out_of_date = Merge Failed: Whilst generating the merge, the base was updated. Hint: Try again.
-pulls.push_rejected = Merge Failed: The push was rejected with the following message:<br>%s<br>Review the githooks for this repository
+pulls.push_rejected = Merge Failed: The push was rejected. Review the githooks for this repository. 
+pulls.push_rejected_summary = Full Rejection Message
 pulls.push_rejected_no_message = Merge Failed: The push was rejected but there was no remote message.<br>Review the githooks for this repository
 pulls.open_unmerged_pull_exists = `You cannot perform a reopen operation because there is a pending pull request (#%d) with identical properties.`
 pulls.status_checking = Some checks are pending

options/locale/locale_it-IT.ini

@@ -1037,8 +1037,7 @@ issues.close_comment_issue=Commenta e Chiudi
 issues.reopen_issue=Riapri
 issues.reopen_comment_issue=Commenta e Riapri
 issues.create_comment=Commento
-issues.closed_at="`chiuso questo probleam <a id=\"%[1]s\" href=\"#%[1]s\">%[2]s</a>`
+issues.closed_at=`chiuso questo probleam <a id="%[1]s" href="#%[1]s">%[2]s</a>`
-Contextrequest"
 issues.reopened_at=`riaperto questo problema <a id="%[1]s" href="#%[1]s">%[2]s</a>`
 issues.commit_ref_at=`ha fatto riferimento a questa issue dal commit <a id="%[1]s" href="#%[1]s">%[2]s</a>`
 issues.ref_issue_from=`<a href="%[3]s">ha fatto riferimento a questo problema %[4]s</a> <a id="%[1]s" href="#%[1]s">%[2]s</a>`

routers/admin/auths.go

@@ -13,6 +13,7 @@ import (
 	"code.gitea.io/gitea/modules/auth"
 	"code.gitea.io/gitea/modules/auth/ldap"
 	"code.gitea.io/gitea/modules/auth/oauth2"
+	"code.gitea.io/gitea/modules/auth/pam"
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/log"
@@ -57,14 +58,20 @@ type dropdownItem struct {
 }
 
 var (
-	authSources = []dropdownItem{
+	authSources = func() []dropdownItem {
-		{models.LoginNames[models.LoginLDAP], models.LoginLDAP},
+		items := []dropdownItem{
-		{models.LoginNames[models.LoginDLDAP], models.LoginDLDAP},
+			{models.LoginNames[models.LoginLDAP], models.LoginLDAP},
-		{models.LoginNames[models.LoginSMTP], models.LoginSMTP},
+			{models.LoginNames[models.LoginDLDAP], models.LoginDLDAP},
-		{models.LoginNames[models.LoginPAM], models.LoginPAM},
+			{models.LoginNames[models.LoginSMTP], models.LoginSMTP},
-		{models.LoginNames[models.LoginOAuth2], models.LoginOAuth2},
+			{models.LoginNames[models.LoginOAuth2], models.LoginOAuth2},
-		{models.LoginNames[models.LoginSSPI], models.LoginSSPI},
+			{models.LoginNames[models.LoginSSPI], models.LoginSSPI},
-	}
+		}
+		if pam.Supported {
+			items = append(items, dropdownItem{models.LoginNames[models.LoginPAM], models.LoginPAM})
+		}
+		return items
+	}()
+
 	securityProtocols = []dropdownItem{
 		{models.SecurityProtocolNames[ldap.SecurityProtocolUnencrypted], ldap.SecurityProtocolUnencrypted},
 		{models.SecurityProtocolNames[ldap.SecurityProtocolLDAPS], ldap.SecurityProtocolLDAPS},

routers/admin/users.go

@@ -129,6 +129,9 @@ func NewUserPost(ctx *context.Context, form auth.AdminCreateUserForm) {
 		case models.IsErrEmailAlreadyUsed(err):
 			ctx.Data["Err_Email"] = true
 			ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplUserNew, &form)
+		case models.IsErrEmailInvalid(err):
+			ctx.Data["Err_Email"] = true
+			ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplUserNew, &form)
 		case models.IsErrNameReserved(err):
 			ctx.Data["Err_UserName"] = true
 			ctx.RenderWithErr(ctx.Tr("user.form.name_reserved", err.(models.ErrNameReserved).Name), tplUserNew, &form)
@@ -277,6 +280,9 @@ func EditUserPost(ctx *context.Context, form auth.AdminEditUserForm) {
 		if models.IsErrEmailAlreadyUsed(err) {
 			ctx.Data["Err_Email"] = true
 			ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplUserEdit, &form)
+		} else if models.IsErrEmailInvalid(err) {
+			ctx.Data["Err_Email"] = true
+			ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplUserEdit, &form)
 		} else {
 			ctx.ServerError("UpdateUser", err)
 		}

routers/admin/users_test.go

@@ -87,3 +87,33 @@ func TestNewUserPost_MustChangePasswordFalse(t *testing.T) {
 	assert.Equal(t, email, u.Email)
 	assert.False(t, u.MustChangePassword)
 }
+
+func TestNewUserPost_InvalidEmail(t *testing.T) {
+
+	models.PrepareTestEnv(t)
+	ctx := test.MockContext(t, "admin/users/new")
+
+	u := models.AssertExistsAndLoadBean(t, &models.User{
+		IsAdmin: true,
+		ID:      2,
+	}).(*models.User)
+
+	ctx.User = u
+
+	username := "gitea"
+	email := "gitea@gitea.io\r\n"
+
+	form := auth.AdminCreateUserForm{
+		LoginType:          "local",
+		LoginName:          "local",
+		UserName:           username,
+		Email:              email,
+		Password:           "abc123ABC!=$",
+		SendNotify:         false,
+		MustChangePassword: false,
+	}
+
+	NewUserPost(ctx, form)
+
+	assert.NotEmpty(t, ctx.Flash.ErrorMsg)
+}

routers/api/v1/admin/user.go

@@ -101,6 +101,7 @@ func CreateUser(ctx *context.APIContext, form api.CreateUserOption) {
 			models.IsErrEmailAlreadyUsed(err) ||
 			models.IsErrNameReserved(err) ||
 			models.IsErrNameCharsNotAllowed(err) ||
+			models.IsErrEmailInvalid(err) ||
 			models.IsErrNamePatternNotAllowed(err) {
 			ctx.Error(http.StatusUnprocessableEntity, "", err)
 		} else {
@@ -208,7 +209,7 @@ func EditUser(ctx *context.APIContext, form api.EditUserOption) {
 	}
 
 	if err := models.UpdateUser(u); err != nil {
-		if models.IsErrEmailAlreadyUsed(err) {
+		if models.IsErrEmailAlreadyUsed(err) || models.IsErrEmailInvalid(err) {
 			ctx.Error(http.StatusUnprocessableEntity, "", err)
 		} else {
 			ctx.Error(http.StatusInternalServerError, "UpdateUser", err)

routers/api/v1/api.go

@@ -191,14 +191,14 @@ func reqToken() macaron.Handler {
 			ctx.RequireCSRF()
 			return
 		}
-		ctx.Context.Error(http.StatusUnauthorized)
+		ctx.Error(http.StatusUnauthorized, "reqToken", "token is required")
 	}
 }
 
 func reqBasicAuth() macaron.Handler {
 	return func(ctx *context.APIContext) {
 		if !ctx.Context.IsBasicAuth {
-			ctx.Context.Error(http.StatusUnauthorized)
+			ctx.Error(http.StatusUnauthorized, "reqBasicAuth", "basic auth required")
 			return
 		}
 		ctx.CheckForOTP()
@@ -207,9 +207,9 @@ func reqBasicAuth() macaron.Handler {
 
 // reqSiteAdmin user should be the site admin
 func reqSiteAdmin() macaron.Handler {
-	return func(ctx *context.Context) {
+	return func(ctx *context.APIContext) {
 		if !ctx.IsUserSiteAdmin() {
-			ctx.Error(http.StatusForbidden)
+			ctx.Error(http.StatusForbidden, "reqSiteAdmin", "user should be the site admin")
 			return
 		}
 	}
@@ -217,9 +217,9 @@ func reqSiteAdmin() macaron.Handler {
 
 // reqOwner user should be the owner of the repo or site admin.
 func reqOwner() macaron.Handler {
-	return func(ctx *context.Context) {
+	return func(ctx *context.APIContext) {
 		if !ctx.IsUserRepoOwner() && !ctx.IsUserSiteAdmin() {
-			ctx.Error(http.StatusForbidden)
+			ctx.Error(http.StatusForbidden, "reqOwner", "user should be the owner of the repo")
 			return
 		}
 	}
@@ -227,9 +227,9 @@ func reqOwner() macaron.Handler {
 
 // reqAdmin user should be an owner or a collaborator with admin write of a repository, or site admin
 func reqAdmin() macaron.Handler {
-	return func(ctx *context.Context) {
+	return func(ctx *context.APIContext) {
 		if !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() {
-			ctx.Error(http.StatusForbidden)
+			ctx.Error(http.StatusForbidden, "reqAdmin", "user should be an owner or a collaborator with admin write of a repository")
 			return
 		}
 	}
@@ -237,9 +237,9 @@ func reqAdmin() macaron.Handler {
 
 // reqRepoWriter user should have a permission to write to a repo, or be a site admin
 func reqRepoWriter(unitTypes ...models.UnitType) macaron.Handler {
-	return func(ctx *context.Context) {
+	return func(ctx *context.APIContext) {
 		if !ctx.IsUserRepoWriter(unitTypes) && !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() {
-			ctx.Error(http.StatusForbidden)
+			ctx.Error(http.StatusForbidden, "reqRepoWriter", "user should have a permission to write to a repo")
 			return
 		}
 	}
@@ -247,9 +247,9 @@ func reqRepoWriter(unitTypes ...models.UnitType) macaron.Handler {
 
 // reqRepoReader user should have specific read permission or be a repo admin or a site admin
 func reqRepoReader(unitType models.UnitType) macaron.Handler {
-	return func(ctx *context.Context) {
+	return func(ctx *context.APIContext) {
 		if !ctx.IsUserRepoReaderSpecific(unitType) && !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() {
-			ctx.Error(http.StatusForbidden)
+			ctx.Error(http.StatusForbidden, "reqRepoReader", "user should have specific read permission or be a repo admin or a site admin")
 			return
 		}
 	}
@@ -257,9 +257,9 @@ func reqRepoReader(unitType models.UnitType) macaron.Handler {
 
 // reqAnyRepoReader user should have any permission to read repository or permissions of site admin
 func reqAnyRepoReader() macaron.Handler {
-	return func(ctx *context.Context) {
+	return func(ctx *context.APIContext) {
 		if !ctx.IsUserRepoReaderAny() && !ctx.IsUserSiteAdmin() {
-			ctx.Error(http.StatusForbidden)
+			ctx.Error(http.StatusForbidden, "reqAnyRepoReader", "user should have any permission to read repository or permissions of site admin")
 			return
 		}
 	}
@@ -502,7 +502,6 @@ func mustNotBeArchived(ctx *context.APIContext) {
 }
 
 // RegisterRoutes registers all v1 APIs routes to web application.
-// FIXME: custom form error response
 func RegisterRoutes(m *macaron.Macaron) {
 	bind := binding.Bind
 
@@ -641,7 +640,7 @@ func RegisterRoutes(m *macaron.Macaron) {
 			m.Group("/:username/:reponame", func() {
 				m.Combo("").Get(reqAnyRepoReader(), repo.Get).
 					Delete(reqToken(), reqOwner(), repo.Delete).
-					Patch(reqToken(), reqAdmin(), bind(api.EditRepoOption{}), context.RepoRef(), repo.Edit)
+					Patch(reqToken(), reqAdmin(), bind(api.EditRepoOption{}), context.RepoRefForAPI(), repo.Edit)
 				m.Post("/transfer", reqOwner(), bind(api.TransferRepoOption{}), repo.Transfer)
 				m.Combo("/notifications").
 					Get(reqToken(), notify.ListRepoNotifications).
@@ -653,7 +652,7 @@ func RegisterRoutes(m *macaron.Macaron) {
 						m.Combo("").Get(repo.GetHook).
 							Patch(bind(api.EditHookOption{}), repo.EditHook).
 							Delete(repo.DeleteHook)
-						m.Post("/tests", context.RepoRef(), repo.TestHook)
+						m.Post("/tests", context.RepoRefForAPI(), repo.TestHook)
 					})
 					m.Group("/git", func() {
 						m.Combo("").Get(repo.ListGitHooks)
@@ -670,14 +669,14 @@ func RegisterRoutes(m *macaron.Macaron) {
 						Put(reqAdmin(), bind(api.AddCollaboratorOption{}), repo.AddCollaborator).
 						Delete(reqAdmin(), repo.DeleteCollaborator)
 				}, reqToken())
-				m.Get("/raw/*", context.RepoRefByType(context.RepoRefAny), reqRepoReader(models.UnitTypeCode), repo.GetRawFile)
+				m.Get("/raw/*", context.RepoRefForAPI(), reqRepoReader(models.UnitTypeCode), repo.GetRawFile)
 				m.Get("/archive/*", reqRepoReader(models.UnitTypeCode), repo.GetArchive)
 				m.Combo("/forks").Get(repo.ListForks).
 					Post(reqToken(), reqRepoReader(models.UnitTypeCode), bind(api.CreateForkOption{}), repo.CreateFork)
 				m.Group("/branches", func() {
 					m.Get("", repo.ListBranches)
-					m.Get("/*", context.RepoRefByType(context.RepoRefBranch), repo.GetBranch)
+					m.Get("/*", repo.GetBranch)
-					m.Delete("/*", reqRepoWriter(models.UnitTypeCode), context.RepoRefByType(context.RepoRefBranch), repo.DeleteBranch)
+					m.Delete("/*", context.ReferencesGitRepo(false), reqRepoWriter(models.UnitTypeCode), repo.DeleteBranch)
 					m.Post("", reqRepoWriter(models.UnitTypeCode), bind(api.CreateBranchRepoOption{}), repo.CreateBranch)
 				}, reqRepoReader(models.UnitTypeCode))
 				m.Group("/branch_protections", func() {
@@ -802,7 +801,7 @@ func RegisterRoutes(m *macaron.Macaron) {
 					})
 				}, reqRepoReader(models.UnitTypeReleases))
 				m.Post("/mirror-sync", reqToken(), reqRepoWriter(models.UnitTypeCode), repo.MirrorSync)
-				m.Get("/editorconfig/:filename", context.RepoRef(), reqRepoReader(models.UnitTypeCode), repo.GetEditorconfig)
+				m.Get("/editorconfig/:filename", context.RepoRefForAPI(), reqRepoReader(models.UnitTypeCode), repo.GetEditorconfig)
 				m.Group("/pulls", func() {
 					m.Combo("").Get(bind(api.ListPullRequestsOptions{}), repo.ListPullRequests).
 						Post(reqToken(), mustNotBeArchived, bind(api.CreatePullRequestOption{}), repo.CreatePullRequest)
@@ -847,9 +846,9 @@ func RegisterRoutes(m *macaron.Macaron) {
 					})
 					m.Get("/refs", repo.GetGitAllRefs)
 					m.Get("/refs/*", repo.GetGitRefs)
-					m.Get("/trees/:sha", context.RepoRef(), repo.GetTree)
+					m.Get("/trees/:sha", context.RepoRefForAPI(), repo.GetTree)
-					m.Get("/blobs/:sha", context.RepoRef(), repo.GetBlob)
+					m.Get("/blobs/:sha", context.RepoRefForAPI(), repo.GetBlob)
-					m.Get("/tags/:sha", context.RepoRef(), repo.GetTag)
+					m.Get("/tags/:sha", context.RepoRefForAPI(), repo.GetTag)
 				}, reqRepoReader(models.UnitTypeCode))
 				m.Group("/contents", func() {
 					m.Get("", repo.GetContentsList)

routers/api/v1/notify/repo.go

@@ -101,7 +101,7 @@ func ListRepoNotifications(ctx *context.APIContext) {
 
 	before, since, err := utils.GetQueryBeforeSince(ctx)
 	if err != nil {
-		ctx.InternalServerError(err)
+		ctx.Error(http.StatusUnprocessableEntity, "GetQueryBeforeSince", err)
 		return
 	}
 	opts := models.FindNotificationOptions{

routers/api/v1/notify/user.go

@@ -63,7 +63,7 @@ func ListNotifications(ctx *context.APIContext) {
 
 	before, since, err := utils.GetQueryBeforeSince(ctx)
 	if err != nil {
-		ctx.InternalServerError(err)
+		ctx.Error(http.StatusUnprocessableEntity, "GetQueryBeforeSince", err)
 		return
 	}
 	opts := models.FindNotificationOptions{

routers/api/v1/repo/branch.go

@@ -46,15 +46,12 @@ func GetBranch(ctx *context.APIContext) {
 	// responses:
 	//   "200":
 	//     "$ref": "#/responses/Branch"
+	//   "404":
+	//     "$ref": "#/responses/notFound"
 
-	if ctx.Repo.TreePath != "" {
+	branchName := ctx.Params("*")
-		// if TreePath != "", then URL contained extra slashes
+
-		// (i.e. "master/subbranch" instead of "master"), so branch does
+	branch, err := repo_module.GetBranch(ctx.Repo.Repository, branchName)
-		// not exist
-		ctx.NotFound()
-		return
-	}
-	branch, err := repo_module.GetBranch(ctx.Repo.Repository, ctx.Repo.BranchName)
 	if err != nil {
 		if git.IsErrBranchNotExist(err) {
 			ctx.NotFound(err)
@@ -70,7 +67,7 @@ func GetBranch(ctx *context.APIContext) {
 		return
 	}
 
-	branchProtection, err := ctx.Repo.Repository.GetBranchProtection(ctx.Repo.BranchName)
+	branchProtection, err := ctx.Repo.Repository.GetBranchProtection(branchName)
 	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "GetBranchProtection", err)
 		return
@@ -113,21 +110,17 @@ func DeleteBranch(ctx *context.APIContext) {
 	//     "$ref": "#/responses/empty"
 	//   "403":
 	//     "$ref": "#/responses/error"
+	//   "404":
+	//     "$ref": "#/responses/notFound"
 
-	if ctx.Repo.TreePath != "" {
+	branchName := ctx.Params("*")
-		// if TreePath != "", then URL contained extra slashes
-		// (i.e. "master/subbranch" instead of "master"), so branch does
-		// not exist
-		ctx.NotFound()
-		return
-	}
 
-	if ctx.Repo.Repository.DefaultBranch == ctx.Repo.BranchName {
+	if ctx.Repo.Repository.DefaultBranch == branchName {
 		ctx.Error(http.StatusForbidden, "DefaultBranch", fmt.Errorf("can not delete default branch"))
 		return
 	}
 
-	isProtected, err := ctx.Repo.Repository.IsProtectedBranch(ctx.Repo.BranchName, ctx.User)
+	isProtected, err := ctx.Repo.Repository.IsProtectedBranch(branchName, ctx.User)
 	if err != nil {
 		ctx.InternalServerError(err)
 		return
@@ -137,7 +130,7 @@ func DeleteBranch(ctx *context.APIContext) {
 		return
 	}
 
-	branch, err := repo_module.GetBranch(ctx.Repo.Repository, ctx.Repo.BranchName)
+	branch, err := repo_module.GetBranch(ctx.Repo.Repository, branchName)
 	if err != nil {
 		if git.IsErrBranchNotExist(err) {
 			ctx.NotFound(err)
@@ -153,7 +146,7 @@ func DeleteBranch(ctx *context.APIContext) {
 		return
 	}
 
-	if err := ctx.Repo.GitRepo.DeleteBranch(ctx.Repo.BranchName, git.DeleteBranchOptions{
+	if err := ctx.Repo.GitRepo.DeleteBranch(branchName, git.DeleteBranchOptions{
 		Force: true,
 	}); err != nil {
 		ctx.Error(http.StatusInternalServerError, "DeleteBranch", err)
@@ -163,7 +156,7 @@ func DeleteBranch(ctx *context.APIContext) {
 	// Don't return error below this
 	if err := repo_service.PushUpdate(
 		&repo_service.PushUpdateOptions{
-			RefFullName:  git.BranchPrefix + ctx.Repo.BranchName,
+			RefFullName:  git.BranchPrefix + branchName,
 			OldCommitID:  c.ID.String(),
 			NewCommitID:  git.EmptySHA,
 			PusherID:     ctx.User.ID,
@@ -174,7 +167,7 @@ func DeleteBranch(ctx *context.APIContext) {
 		log.Error("Update: %v", err)
 	}
 
-	if err := ctx.Repo.Repository.AddDeletedBranch(ctx.Repo.BranchName, c.ID.String(), ctx.User.ID); err != nil {
+	if err := ctx.Repo.Repository.AddDeletedBranch(branchName, c.ID.String(), ctx.User.ID); err != nil {
 		log.Warn("AddDeletedBranch: %v", err)
 	}
 

routers/api/v1/repo/issue_comment.go

@@ -56,7 +56,7 @@ func ListIssueComments(ctx *context.APIContext) {
 
 	before, since, err := utils.GetQueryBeforeSince(ctx)
 	if err != nil {
-		ctx.Error(http.StatusInternalServerError, "GetQueryBeforeSince", err)
+		ctx.Error(http.StatusUnprocessableEntity, "GetQueryBeforeSince", err)
 		return
 	}
 	issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
@@ -132,7 +132,7 @@ func ListRepoIssueComments(ctx *context.APIContext) {
 
 	before, since, err := utils.GetQueryBeforeSince(ctx)
 	if err != nil {
-		ctx.Error(http.StatusInternalServerError, "GetQueryBeforeSince", err)
+		ctx.Error(http.StatusUnprocessableEntity, "GetQueryBeforeSince", err)
 		return
 	}
 

routers/api/v1/repo/issue_reaction.go

@@ -56,7 +56,11 @@ func GetIssueCommentReactions(ctx *context.APIContext) {
 		return
 	}
 
-	if !ctx.Repo.CanRead(models.UnitTypeIssues) {
+	if err := comment.LoadIssue(); err != nil {
+		ctx.Error(http.StatusInternalServerError, "comment.LoadIssue", err)
+	}
+
+	if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) {
 		ctx.Error(http.StatusForbidden, "GetIssueCommentReactions", errors.New("no permission to get reactions"))
 		return
 	}
@@ -270,7 +274,7 @@ func GetIssueReactions(ctx *context.APIContext) {
 		return
 	}
 
-	if !ctx.Repo.CanRead(models.UnitTypeIssues) {
+	if !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull) {
 		ctx.Error(http.StatusForbidden, "GetIssueReactions", errors.New("no permission to get reactions"))
 		return
 	}

routers/api/v1/repo/issue_tracked_time.go

@@ -86,7 +86,7 @@ func ListTrackedTimes(ctx *context.APIContext) {
 	}
 
 	if opts.CreatedBeforeUnix, opts.CreatedAfterUnix, err = utils.GetQueryBeforeSince(ctx); err != nil {
-		ctx.InternalServerError(err)
+		ctx.Error(http.StatusUnprocessableEntity, "GetQueryBeforeSince", err)
 		return
 	}
 
@@ -491,7 +491,7 @@ func ListTrackedTimesByRepository(ctx *context.APIContext) {
 
 	var err error
 	if opts.CreatedBeforeUnix, opts.CreatedAfterUnix, err = utils.GetQueryBeforeSince(ctx); err != nil {
-		ctx.InternalServerError(err)
+		ctx.Error(http.StatusUnprocessableEntity, "GetQueryBeforeSince", err)
 		return
 	}
 
@@ -554,7 +554,7 @@ func ListMyTrackedTimes(ctx *context.APIContext) {
 
 	var err error
 	if opts.CreatedBeforeUnix, opts.CreatedAfterUnix, err = utils.GetQueryBeforeSince(ctx); err != nil {
-		ctx.InternalServerError(err)
+		ctx.Error(http.StatusUnprocessableEntity, "GetQueryBeforeSince", err)
 		return
 	}
 

routers/api/v1/repo/migrate.go

@@ -212,6 +212,8 @@ func handleMigrateError(ctx *context.APIContext, repoOwner *models.User, remoteA
 		ctx.Error(http.StatusUnprocessableEntity, "", fmt.Sprintf("The username '%s' contains invalid characters.", err.(models.ErrNameCharsNotAllowed).Name))
 	case models.IsErrNamePatternNotAllowed(err):
 		ctx.Error(http.StatusUnprocessableEntity, "", fmt.Sprintf("The pattern '%s' is not allowed in a username.", err.(models.ErrNamePatternNotAllowed).Pattern))
+	case models.IsErrMigrationNotAllowed(err):
+		ctx.Error(http.StatusUnprocessableEntity, "", err)
 	default:
 		err = util.URLSanitizedError(err, remoteAddr)
 		if strings.Contains(err.Error(), "Authentication failed") ||

routers/api/v1/repo/pull.go

@@ -284,6 +284,12 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption
 	//   "422":
 	//     "$ref": "#/responses/validationError"
 
+	if form.Head == form.Base {
+		ctx.Error(http.StatusUnprocessableEntity, "BaseHeadSame",
+			"Invalid PullRequest: There are no changes between the head and the base")
+		return
+	}
+
 	var (
 		repo        = ctx.Repo.Repository
 		labelIDs    []int64

routers/api/v1/user/email.go

@@ -5,6 +5,7 @@
 package user
 
 import (
+	"fmt"
 	"net/http"
 
 	"code.gitea.io/gitea/models"
@@ -78,6 +79,9 @@ func AddEmail(ctx *context.APIContext, form api.CreateEmailOption) {
 	if err := models.AddEmailAddresses(emails); err != nil {
 		if models.IsErrEmailAlreadyUsed(err) {
 			ctx.Error(http.StatusUnprocessableEntity, "", "Email address has been used: "+err.(models.ErrEmailAlreadyUsed).Email)
+		} else if models.IsErrEmailInvalid(err) {
+			errMsg := fmt.Sprintf("Email address %s invalid", err.(models.ErrEmailInvalid).Email)
+			ctx.Error(http.StatusUnprocessableEntity, "", errMsg)
 		} else {
 			ctx.Error(http.StatusInternalServerError, "AddEmailAddresses", err)
 		}

routers/api/v1/utils/utils.go

@@ -5,6 +5,7 @@
 package utils
 
 import (
+	"net/url"
 	"strings"
 	"time"
 
@@ -15,30 +16,49 @@ import (
 
 // GetQueryBeforeSince return parsed time (unix format) from URL query's before and since
 func GetQueryBeforeSince(ctx *context.APIContext) (before, since int64, err error) {
-	qCreatedBefore := strings.Trim(ctx.Query("before"), " ")
+	qCreatedBefore, err := prepareQueryArg(ctx, "before")
-	if qCreatedBefore != "" {
+	if err != nil {
-		createdBefore, err := time.Parse(time.RFC3339, qCreatedBefore)
+		return 0, 0, err
-		if err != nil {
-			return 0, 0, err
-		}
-		if !createdBefore.IsZero() {
-			before = createdBefore.Unix()
-		}
 	}
 
-	qCreatedAfter := strings.Trim(ctx.Query("since"), " ")
+	qCreatedSince, err := prepareQueryArg(ctx, "since")
-	if qCreatedAfter != "" {
+	if err != nil {
-		createdAfter, err := time.Parse(time.RFC3339, qCreatedAfter)
+		return 0, 0, err
-		if err != nil {
+	}
-			return 0, 0, err
+
-		}
+	before, err = parseTime(qCreatedBefore)
-		if !createdAfter.IsZero() {
+	if err != nil {
-			since = createdAfter.Unix()
+		return 0, 0, err
-		}
+	}
+
+	since, err = parseTime(qCreatedSince)
+	if err != nil {
+		return 0, 0, err
 	}
 	return before, since, nil
 }
 
+// parseTime parse time and return unix timestamp
+func parseTime(value string) (int64, error) {
+	if len(value) != 0 {
+		t, err := time.Parse(time.RFC3339, value)
+		if err != nil {
+			return 0, err
+		}
+		if !t.IsZero() {
+			return t.Unix(), nil
+		}
+	}
+	return 0, nil
+}
+
+// prepareQueryArg unescape and trim a query arg
+func prepareQueryArg(ctx *context.APIContext, name string) (value string, err error) {
+	value, err = url.PathUnescape(ctx.Query(name))
+	value = strings.Trim(value, " ")
+	return
+}
+
 // GetListOptions returns list options using the page and limit parameters
 func GetListOptions(ctx *context.APIContext) models.ListOptions {
 	return models.ListOptions{

routers/init.go

@@ -24,6 +24,7 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/markup"
 	"code.gitea.io/gitea/modules/markup/external"
+	repo_migrations "code.gitea.io/gitea/modules/migrations"
 	"code.gitea.io/gitea/modules/notification"
 	"code.gitea.io/gitea/modules/options"
 	"code.gitea.io/gitea/modules/setting"
@@ -43,12 +44,14 @@ import (
 
 func checkRunMode() {
 	switch setting.Cfg.Section("").Key("RUN_MODE").String() {
-	case "prod":
+	case "dev":
+		git.Debug = true
+	case "test":
+		git.Debug = true
+	default:
 		macaron.Env = macaron.PROD
 		macaron.ColorLog = false
 		setting.ProdMode = true
-	default:
-		git.Debug = true
 	}
 	log.Info("Run Mode: %s", strings.Title(macaron.Env))
 }
@@ -172,6 +175,10 @@ func GlobalInit(ctx context.Context) {
 	}
 	checkRunMode()
 
+	if err := repo_migrations.Init(); err != nil {
+		log.Fatal("Failed to initialize repository migrations: %v", err)
+	}
+
 	// Now because Install will re-run GlobalInit once it has set InstallLock
 	// we can't tell if the ssh port will remain unused until that's done.
 	// However, see FIXME comment in install.go

routers/private/serv.go

@@ -61,6 +61,12 @@ func ServNoCommand(ctx *macaron.Context) {
 			})
 			return
 		}
+		if !user.IsActive || user.ProhibitLogin {
+			ctx.JSON(http.StatusForbidden, map[string]interface{}{
+				"err": "Your account is disabled.",
+			})
+			return
+		}
 		results.Owner = user
 	}
 	ctx.JSON(http.StatusOK, &results)
@@ -98,9 +104,28 @@ func ServCommand(ctx *macaron.Context) {
 		results.RepoName = repoName[:len(repoName)-5]
 	}
 
+	owner, err := models.GetUserByName(results.OwnerName)
+	if err != nil {
+		log.Error("Unable to get repository owner: %s/%s Error: %v", results.OwnerName, results.RepoName, err)
+		ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
+			"results": results,
+			"type":    "InternalServerError",
+			"err":     fmt.Sprintf("Unable to get repository owner: %s/%s %v", results.OwnerName, results.RepoName, err),
+		})
+		return
+	}
+	if !owner.IsOrganization() && !owner.IsActive {
+		ctx.JSON(http.StatusForbidden, map[string]interface{}{
+			"results": results,
+			"type":    "ForbiddenError",
+			"err":     "Repository cannot be accessed, you could retry it later",
+		})
+		return
+	}
+
 	// Now get the Repository and set the results section
 	repoExist := true
-	repo, err := models.GetRepositoryByOwnerAndName(results.OwnerName, results.RepoName)
+	repo, err := models.GetRepositoryByName(owner.ID, results.RepoName)
 	if err != nil {
 		if models.IsErrRepoNotExist(err) {
 			repoExist = false
@@ -127,6 +152,7 @@ func ServCommand(ctx *macaron.Context) {
 	}
 
 	if repoExist {
+		repo.Owner = owner
 		repo.OwnerName = ownerName
 		results.RepoID = repo.ID
 
@@ -217,15 +243,6 @@ func ServCommand(ctx *macaron.Context) {
 		// so for now use the owner of the repository
 		results.UserName = results.OwnerName
 		results.UserID = repo.OwnerID
-		if err = repo.GetOwner(); err != nil {
-			log.Error("Unable to get owner for repo %-v. Error: %v", repo, err)
-			ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
-				"results": results,
-				"type":    "InternalServerError",
-				"err":     fmt.Sprintf("Unable to get owner for repo: %s/%s.", results.OwnerName, results.RepoName),
-			})
-			return
-		}
 		if !repo.Owner.KeepEmailPrivate {
 			results.UserEmail = repo.Owner.Email
 		}
@@ -250,6 +267,14 @@ func ServCommand(ctx *macaron.Context) {
 			})
 			return
 		}
+
+		if !user.IsActive || user.ProhibitLogin {
+			ctx.JSON(http.StatusForbidden, map[string]interface{}{
+				"err": "Your account is disabled.",
+			})
+			return
+		}
+
 		results.UserName = user.Name
 		if !user.KeepEmailPrivate {
 			results.UserEmail = user.Email

routers/repo/branch.go

@@ -355,7 +355,16 @@ func CreateBranch(ctx *context.Context, form auth.NewBranchForm) {
 			if len(e.Message) == 0 {
 				ctx.Flash.Error(ctx.Tr("repo.editor.push_rejected_no_message"))
 			} else {
-				ctx.Flash.Error(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(e.Message)))
+				flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+					"Message": ctx.Tr("repo.editor.push_rejected"),
+					"Summary": ctx.Tr("repo.editor.push_rejected_summary"),
+					"Details": utils.SanitizeFlashErrorString(e.Message),
+				})
+				if err != nil {
+					ctx.ServerError("UpdatePullRequest.HTMLString", err)
+					return
+				}
+				ctx.Flash.Error(flashError)
 			}
 			ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL())
 			return

routers/repo/editor.go

@@ -293,10 +293,28 @@ func editFilePost(ctx *context.Context, form auth.EditRepoFileForm, isNewFile bo
 			if len(errPushRej.Message) == 0 {
 				ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplEditFile, &form)
 			} else {
-				ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(errPushRej.Message)), tplEditFile, &form)
+				flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+					"Message": ctx.Tr("repo.editor.push_rejected"),
+					"Summary": ctx.Tr("repo.editor.push_rejected_summary"),
+					"Details": utils.SanitizeFlashErrorString(errPushRej.Message),
+				})
+				if err != nil {
+					ctx.ServerError("editFilePost.HTMLString", err)
+					return
+				}
+				ctx.RenderWithErr(flashError, tplEditFile, &form)
 			}
 		} else {
-			ctx.RenderWithErr(ctx.Tr("repo.editor.fail_to_update_file", form.TreePath, utils.SanitizeFlashErrorString(err.Error())), tplEditFile, &form)
+			flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+				"Message": ctx.Tr("repo.editor.fail_to_update_file", form.TreePath),
+				"Summary": ctx.Tr("repo.editor.fail_to_update_file_summary"),
+				"Details": utils.SanitizeFlashErrorString(err.Error()),
+			})
+			if err != nil {
+				ctx.ServerError("editFilePost.HTMLString", err)
+				return
+			}
+			ctx.RenderWithErr(flashError, tplEditFile, &form)
 		}
 	}
 
@@ -464,7 +482,16 @@ func DeleteFilePost(ctx *context.Context, form auth.DeleteRepoFileForm) {
 			if len(errPushRej.Message) == 0 {
 				ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplDeleteFile, &form)
 			} else {
-				ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(errPushRej.Message)), tplDeleteFile, &form)
+				flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+					"Message": ctx.Tr("repo.editor.push_rejected"),
+					"Summary": ctx.Tr("repo.editor.push_rejected_summary"),
+					"Details": utils.SanitizeFlashErrorString(errPushRej.Message),
+				})
+				if err != nil {
+					ctx.ServerError("DeleteFilePost.HTMLString", err)
+					return
+				}
+				ctx.RenderWithErr(flashError, tplDeleteFile, &form)
 			}
 		} else {
 			ctx.ServerError("DeleteRepoFile", err)
@@ -656,7 +683,16 @@ func UploadFilePost(ctx *context.Context, form auth.UploadRepoFileForm) {
 			if len(errPushRej.Message) == 0 {
 				ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplUploadFile, &form)
 			} else {
-				ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(errPushRej.Message)), tplUploadFile, &form)
+				flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+					"Message": ctx.Tr("repo.editor.push_rejected"),
+					"Summary": ctx.Tr("repo.editor.push_rejected_summary"),
+					"Details": utils.SanitizeFlashErrorString(errPushRej.Message),
+				})
+				if err != nil {
+					ctx.ServerError("UploadFilePost.HTMLString", err)
+					return
+				}
+				ctx.RenderWithErr(flashError, tplUploadFile, &form)
 			}
 		} else {
 			// os.ErrNotExist - upload file missing in the intervening time?!

routers/repo/http.go

@@ -105,6 +105,10 @@ func HTTP(ctx *context.Context) {
 		ctx.NotFoundOrServerError("GetUserByName", models.IsErrUserNotExist, err)
 		return
 	}
+	if !owner.IsOrganization() && !owner.IsActive {
+		ctx.HandleText(http.StatusForbidden, "Repository cannot be accessed. You cannot push or open issues/pull-requests.")
+		return
+	}
 
 	repoExist := true
 	repo, err := models.GetRepositoryByName(owner.ID, reponame)
@@ -244,6 +248,11 @@ func HTTP(ctx *context.Context) {
 			}
 		}
 
+		if !authUser.IsActive || authUser.ProhibitLogin {
+			ctx.HandleText(http.StatusForbidden, "Your account is disabled.")
+			return
+		}
+
 		if repoExist {
 			perm, err := models.GetUserRepoPermission(repo, authUser)
 			if err != nil {

routers/repo/issue.go

@@ -130,6 +130,8 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti
 			posterID = ctx.User.ID
 		case "mentioned":
 			mentionedID = ctx.User.ID
+		case "assigned":
+			assigneeID = ctx.User.ID
 		}
 	}
 
@@ -977,8 +979,27 @@ func commentTag(repo *models.Repository, poster *models.User, issue *models.Issu
 		return models.CommentTagNone, err
 	}
 	if perm.IsOwner() {
-		return models.CommentTagOwner, nil
+		if !poster.IsAdmin {
-	} else if perm.CanWrite(models.UnitTypeCode) {
+			return models.CommentTagOwner, nil
+		}
+
+		ok, err := models.IsUserRealRepoAdmin(repo, poster)
+		if err != nil {
+			return models.CommentTagNone, err
+		}
+
+		if ok {
+			return models.CommentTagOwner, nil
+		}
+
+		if ok, err = repo.IsCollaborator(poster.ID); ok && err == nil {
+			return models.CommentTagWriter, nil
+		}
+
+		return models.CommentTagNone, err
+	}
+
+	if perm.CanWrite(models.UnitTypeCode) {
 		return models.CommentTagWriter, nil
 	}
 

routers/repo/pull.go

@@ -311,7 +311,7 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *models.Issue) *git.C
 	compareInfo, err := ctx.Repo.GitRepo.GetCompareInfo(ctx.Repo.Repository.RepoPath(),
 		pull.MergeBase, pull.GetGitRefName())
 	if err != nil {
-		if strings.Contains(err.Error(), "fatal: Not a valid object name") {
+		if strings.Contains(err.Error(), "fatal: Not a valid object name") || strings.Contains(err.Error(), "unknown revision or path not in the working tree") {
 			ctx.Data["IsPullRequestBroken"] = true
 			ctx.Data["BaseTarget"] = pull.BaseBranch
 			ctx.Data["NumCommits"] = 0
@@ -723,7 +723,16 @@ func UpdatePullRequest(ctx *context.Context) {
 	if err = pull_service.Update(issue.PullRequest, ctx.User, message); err != nil {
 		if models.IsErrMergeConflicts(err) {
 			conflictError := err.(models.ErrMergeConflicts)
-			ctx.Flash.Error(ctx.Tr("repo.pulls.merge_conflict", utils.SanitizeFlashErrorString(conflictError.StdErr), utils.SanitizeFlashErrorString(conflictError.StdOut)))
+			flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+				"Message": ctx.Tr("repo.pulls.merge_conflict"),
+				"Summary": ctx.Tr("repo.pulls.merge_conflict_summary"),
+				"Details": utils.SanitizeFlashErrorString(conflictError.StdErr) + "<br>" + utils.SanitizeFlashErrorString(conflictError.StdOut),
+			})
+			if err != nil {
+				ctx.ServerError("UpdatePullRequest.HTMLString", err)
+				return
+			}
+			ctx.Flash.Error(flashError)
 			ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index))
 			return
 		}
@@ -846,12 +855,30 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
 			return
 		} else if models.IsErrMergeConflicts(err) {
 			conflictError := err.(models.ErrMergeConflicts)
-			ctx.Flash.Error(ctx.Tr("repo.pulls.merge_conflict", utils.SanitizeFlashErrorString(conflictError.StdErr), utils.SanitizeFlashErrorString(conflictError.StdOut)))
+			flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+				"Message": ctx.Tr("repo.editor.merge_conflict"),
+				"Summary": ctx.Tr("repo.editor.merge_conflict_summary"),
+				"Details": utils.SanitizeFlashErrorString(conflictError.StdErr) + "<br>" + utils.SanitizeFlashErrorString(conflictError.StdOut),
+			})
+			if err != nil {
+				ctx.ServerError("MergePullRequest.HTMLString", err)
+				return
+			}
+			ctx.Flash.Error(flashError)
 			ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
 			return
 		} else if models.IsErrRebaseConflicts(err) {
 			conflictError := err.(models.ErrRebaseConflicts)
-			ctx.Flash.Error(ctx.Tr("repo.pulls.rebase_conflict", utils.SanitizeFlashErrorString(conflictError.CommitSHA), utils.SanitizeFlashErrorString(conflictError.StdErr), utils.SanitizeFlashErrorString(conflictError.StdOut)))
+			flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+				"Message": ctx.Tr("repo.pulls.rebase_conflict", utils.SanitizeFlashErrorString(conflictError.CommitSHA)),
+				"Summary": ctx.Tr("repo.pulls.rebase_conflict_summary"),
+				"Details": utils.SanitizeFlashErrorString(conflictError.StdErr) + "<br>" + utils.SanitizeFlashErrorString(conflictError.StdOut),
+			})
+			if err != nil {
+				ctx.ServerError("MergePullRequest.HTMLString", err)
+				return
+			}
+			ctx.Flash.Error(flashError)
 			ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
 			return
 		} else if models.IsErrMergeUnrelatedHistories(err) {
@@ -871,7 +898,16 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
 			if len(message) == 0 {
 				ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message"))
 			} else {
-				ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected", utils.SanitizeFlashErrorString(pushrejErr.Message)))
+				flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+					"Message": ctx.Tr("repo.pulls.push_rejected"),
+					"Summary": ctx.Tr("repo.pulls.push_rejected_summary"),
+					"Details": utils.SanitizeFlashErrorString(pushrejErr.Message),
+				})
+				if err != nil {
+					ctx.ServerError("MergePullRequest.HTMLString", err)
+					return
+				}
+				ctx.Flash.Error(flashError)
 			}
 			ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
 			return
@@ -986,7 +1022,16 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm)
 			if len(message) == 0 {
 				ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message"))
 			} else {
-				ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected", utils.SanitizeFlashErrorString(pushrejErr.Message)))
+				flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+					"Message": ctx.Tr("repo.pulls.push_rejected"),
+					"Summary": ctx.Tr("repo.pulls.push_rejected_summary"),
+					"Details": utils.SanitizeFlashErrorString(pushrejErr.Message),
+				})
+				if err != nil {
+					ctx.ServerError("CompareAndPullRequest.HTMLString", err)
+					return
+				}
+				ctx.Flash.Error(flashError)
 			}
 			ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pullIssue.Index))
 			return

routers/repo/repo.go

@@ -24,7 +24,8 @@ import (
 )
 
 const (
-	tplCreate base.TplName = "repo/create"
+	tplCreate       base.TplName = "repo/create"
+	tplAlertDetails base.TplName = "base/alert_details"
 )
 
 // MustBeNotEmpty render when a repo is a empty git dir
@@ -401,19 +402,3 @@ func Download(ctx *context.Context) {
 
 	ctx.ServeFile(archivePath, ctx.Repo.Repository.Name+"-"+refName+ext)
 }
-
-// Status returns repository's status
-func Status(ctx *context.Context) {
-	task, err := models.GetMigratingTask(ctx.Repo.Repository.ID)
-	if err != nil {
-		ctx.JSON(500, map[string]interface{}{
-			"err": err,
-		})
-		return
-	}
-
-	ctx.JSON(200, map[string]interface{}{
-		"status": ctx.Repo.Repository.Status,
-		"err":    task.Errors,
-	})
-}

routers/routes/routes.go

@@ -7,8 +7,11 @@ package routes
 import (
 	"bytes"
 	"encoding/gob"
+	"errors"
+	"fmt"
 	"io"
 	"net/http"
+	"os"
 	"path"
 	"strings"
 	"text/template"
@@ -125,7 +128,13 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor
 			rPath := strings.TrimPrefix(req.RequestURI, "/"+prefix)
 			u, err := objStore.URL(rPath, path.Base(rPath))
 			if err != nil {
-				ctx.Error(500, err.Error())
+				if os.IsNotExist(err) || errors.Is(err, os.ErrNotExist) {
+					log.Warn("Unable to find %s %s", prefix, rPath)
+					ctx.Error(404, "file not found")
+					return
+				}
+				log.Error("Error whilst getting URL for %s %s. Error: %v", prefix, rPath, err)
+				ctx.Error(500, fmt.Sprintf("Error whilst getting URL for %s %s", prefix, rPath))
 				return
 			}
 			http.Redirect(
@@ -152,7 +161,13 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor
 		//If we have matched and access to release or issue
 		fr, err := objStore.Open(rPath)
 		if err != nil {
-			ctx.Error(500, err.Error())
+			if os.IsNotExist(err) || errors.Is(err, os.ErrNotExist) {
+				log.Warn("Unable to find %s %s", prefix, rPath)
+				ctx.Error(404, "file not found")
+				return
+			}
+			log.Error("Error whilst opening %s %s. Error: %v", prefix, rPath, err)
+			ctx.Error(500, fmt.Sprintf("Error whilst opening %s %s", prefix, rPath))
 			return
 		}
 		defer fr.Close()
@@ -464,6 +479,7 @@ func RegisterRoutes(m *macaron.Macaron) {
 		m.Get("/forgot_password", user.ForgotPasswd)
 		m.Post("/forgot_password", user.ForgotPasswdPost)
 		m.Post("/logout", user.SignOut)
+		m.Get("/task/:task", user.TaskStatus)
 	})
 	// ***** END: User *****
 
@@ -971,8 +987,6 @@ func RegisterRoutes(m *macaron.Macaron) {
 
 		m.Get("/archive/*", repo.MustBeNotEmpty, reqRepoCodeReader, repo.Download)
 
-		m.Get("/status", reqRepoCodeReader, repo.Status)
-
 		m.Group("/branches", func() {
 			m.Get("", repo.Branches)
 		}, repo.MustBeNotEmpty, context.RepoRef(), reqRepoCodeReader)

routers/user/auth.go

@@ -964,6 +964,9 @@ func LinkAccountPostRegister(ctx *context.Context, cpt *captcha.Captcha, form au
 		case models.IsErrEmailAlreadyUsed(err):
 			ctx.Data["Err_Email"] = true
 			ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplLinkAccount, &form)
+		case models.IsErrEmailInvalid(err):
+			ctx.Data["Err_Email"] = true
+			ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplSignUp, &form)
 		case models.IsErrNameReserved(err):
 			ctx.Data["Err_UserName"] = true
 			ctx.RenderWithErr(ctx.Tr("user.form.name_reserved", err.(models.ErrNameReserved).Name), tplLinkAccount, &form)
@@ -1151,6 +1154,9 @@ func SignUpPost(ctx *context.Context, cpt *captcha.Captcha, form auth.RegisterFo
 		case models.IsErrEmailAlreadyUsed(err):
 			ctx.Data["Err_Email"] = true
 			ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplSignUp, &form)
+		case models.IsErrEmailInvalid(err):
+			ctx.Data["Err_Email"] = true
+			ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplSignUp, &form)
 		case models.IsErrNameReserved(err):
 			ctx.Data["Err_UserName"] = true
 			ctx.RenderWithErr(ctx.Tr("user.form.name_reserved", err.(models.ErrNameReserved).Name), tplSignUp, &form)

routers/user/setting/account.go

@@ -179,6 +179,11 @@ func EmailPost(ctx *context.Context, form auth.AddEmailForm) {
 
 			ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplSettingsAccount, &form)
 			return
+		} else if models.IsErrEmailInvalid(err) {
+			loadAccountData(ctx)
+
+			ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplSettingsAccount, &form)
+			return
 		}
 		ctx.ServerError("AddEmailAddress", err)
 		return

routers/user/setting/profile.go

@@ -91,7 +91,6 @@ func ProfilePost(ctx *context.Context, form auth.UpdateProfileForm) {
 	}
 
 	ctx.User.FullName = form.FullName
-	ctx.User.Email = form.Email
 	ctx.User.KeepEmailPrivate = form.KeepEmailPrivate
 	ctx.User.Website = form.Website
 	ctx.User.Location = form.Location
@@ -121,7 +120,11 @@ func ProfilePost(ctx *context.Context, form auth.UpdateProfileForm) {
 func UpdateAvatarSetting(ctx *context.Context, form auth.AvatarForm, ctxUser *models.User) error {
 	ctxUser.UseCustomAvatar = form.Source == auth.AvatarLocal
 	if len(form.Gravatar) > 0 {
-		ctxUser.Avatar = base.EncodeMD5(form.Gravatar)
+		if form.Avatar != nil {
+			ctxUser.Avatar = base.EncodeMD5(form.Gravatar)
+		} else {
+			ctxUser.Avatar = ""
+		}
 		ctxUser.AvatarEmail = form.Gravatar
 	}
 

routers/user/task.go

@@ -0,0 +1,30 @@
+// Copyright 2020 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
+
+import (
+	"code.gitea.io/gitea/models"
+	"code.gitea.io/gitea/modules/context"
+)
+
+// TaskStatus returns task's status
+func TaskStatus(ctx *context.Context) {
+	task, opts, err := models.GetMigratingTaskByID(ctx.ParamsInt64("task"), ctx.User.ID)
+	if err != nil {
+		ctx.JSON(500, map[string]interface{}{
+			"err": err,
+		})
+		return
+	}
+
+	ctx.JSON(200, map[string]interface{}{
+		"status":    task.Status,
+		"err":       task.Errors,
+		"repo-id":   task.RepoID,
+		"repo-name": opts.RepoName,
+		"start":     task.StartTime,
+		"end":       task.EndTime,
+	})
+}

routers/utils/utils.go

@@ -41,12 +41,6 @@ func IsValidSlackChannel(channelName string) bool {
 
 // SanitizeFlashErrorString will sanitize a flash error string
 func SanitizeFlashErrorString(x string) string {
-	runes := []rune(x)
-
-	if len(runes) > 512 {
-		x = "..." + string(runes[len(runes)-512:])
-	}
-
 	return strings.ReplaceAll(html.EscapeString(x), "\n", "<br>")
 }
 

services/gitdiff/gitdiff.go

@@ -181,64 +181,87 @@ var (
 	removedCodePrefix = []byte(`<span class="removed-code">`)
 	codeTagSuffix     = []byte(`</span>`)
 )
-var addSpanRegex = regexp.MustCompile(`<span [class="[a-z]*]*$`)
+var trailingSpanRegex = regexp.MustCompile(`<span\s*[[:alpha:]="]*?[>]?$`)
+var entityRegex = regexp.MustCompile(`&[#]*?[0-9[:alpha:]]*$`)
+
+// shouldWriteInline represents combinations where we manually write inline changes
+func shouldWriteInline(diff diffmatchpatch.Diff, lineType DiffLineType) bool {
+	if true &&
+		diff.Type == diffmatchpatch.DiffEqual ||
+		diff.Type == diffmatchpatch.DiffInsert && lineType == DiffLineAdd ||
+		diff.Type == diffmatchpatch.DiffDelete && lineType == DiffLineDel {
+		return true
+	}
+	return false
+}
 
 func diffToHTML(fileName string, diffs []diffmatchpatch.Diff, lineType DiffLineType) template.HTML {
 	buf := bytes.NewBuffer(nil)
-	var addSpan string
+	match := ""
-	for i := range diffs {
+
+	for _, diff := range diffs {
+		if shouldWriteInline(diff, lineType) {
+			if len(match) > 0 {
+				diff.Text = match + diff.Text
+				match = ""
+			}
+			// Chroma HTML syntax highlighting is done before diffing individual lines in order to maintain consistency.
+			// Since inline changes might split in the middle of a chroma span tag or HTML entity, make we manually put it back together
+			// before writing so we don't try insert added/removed code spans in the middle of one of those
+			// and create broken HTML. This is done by moving incomplete HTML forward until it no longer matches our pattern of
+			// a line ending with an incomplete HTML entity or partial/opening <span>.
+
+			// EX:
+			// diffs[{Type: dmp.DiffDelete, Text: "language</span><span "},
+			// {Type: dmp.DiffEqual, Text: "c"},
+			// {Type: dmp.DiffDelete, Text: "lass="p">}]
+
+			// After first iteration
+			// diffs[{Type: dmp.DiffDelete, Text: "language</span>"}, //write out
+			// {Type: dmp.DiffEqual, Text: "<span c"},
+			// {Type: dmp.DiffDelete, Text: "lass="p">,</span>}]
+
+			// After second iteration
+			// {Type: dmp.DiffEqual, Text: ""}, // write out
+			// {Type: dmp.DiffDelete, Text: "<span class="p">,</span>}]
+
+			// Final
+			// {Type: dmp.DiffDelete, Text: "<span class="p">,</span>}]
+			// end up writing <span class="removed-code"><span class="p">,</span></span>
+			// Instead of <span class="removed-code">lass="p",</span></span>
+
+			m := trailingSpanRegex.FindStringSubmatchIndex(diff.Text)
+			if m != nil {
+				match = diff.Text[m[0]:m[1]]
+				diff.Text = strings.TrimSuffix(diff.Text, match)
+			}
+			m = entityRegex.FindStringSubmatchIndex(diff.Text)
+			if m != nil {
+				match = diff.Text[m[0]:m[1]]
+				diff.Text = strings.TrimSuffix(diff.Text, match)
+			}
+			// Print an existing closing span first before opening added/remove-code span so it doesn't unintentionally close it
+			if strings.HasPrefix(diff.Text, "</span>") {
+				buf.WriteString("</span>")
+				diff.Text = strings.TrimPrefix(diff.Text, "</span>")
+			}
+			// If we weren't able to fix it then this should avoid broken HTML by not inserting more spans below
+			// The previous/next diff section will contain the rest of the tag that is missing here
+			if strings.Count(diff.Text, "<") != strings.Count(diff.Text, ">") {
+				buf.WriteString(diff.Text)
+				continue
+			}
+		}
 		switch {
-		case diffs[i].Type == diffmatchpatch.DiffEqual:
+		case diff.Type == diffmatchpatch.DiffEqual:
-			// Looking for the case where our 3rd party diff library previously detected a string difference
+			buf.WriteString(diff.Text)
-			// in the middle of a span class because we highlight them first. This happens when added/deleted code
+		case diff.Type == diffmatchpatch.DiffInsert && lineType == DiffLineAdd:
-			// also changes the chroma class name, either partially or fully. If found, just move the openining span code forward into the next section
-			// see TestDiffToHTML for examples
-			if len(addSpan) > 0 {
-				diffs[i].Text = addSpan + diffs[i].Text
-				addSpan = ""
-			}
-			m := addSpanRegex.FindStringSubmatchIndex(diffs[i].Text)
-			if m != nil {
-				addSpan = diffs[i].Text[m[0]:m[1]]
-				buf.WriteString(strings.TrimSuffix(diffs[i].Text, addSpan))
-			} else {
-				addSpan = ""
-				buf.WriteString(getLineContent(diffs[i].Text))
-			}
-		case diffs[i].Type == diffmatchpatch.DiffInsert && lineType == DiffLineAdd:
-			if len(addSpan) > 0 {
-				diffs[i].Text = addSpan + diffs[i].Text
-				addSpan = ""
-			}
-			// Print existing closing span first before opening added-code span so it doesn't unintentionally close it
-			if strings.HasPrefix(diffs[i].Text, "</span>") {
-				buf.WriteString("</span>")
-				diffs[i].Text = strings.TrimPrefix(diffs[i].Text, "</span>")
-			}
-			m := addSpanRegex.FindStringSubmatchIndex(diffs[i].Text)
-			if m != nil {
-				addSpan = diffs[i].Text[m[0]:m[1]]
-				diffs[i].Text = strings.TrimSuffix(diffs[i].Text, addSpan)
-			}
 			buf.Write(addedCodePrefix)
-			buf.WriteString(getLineContent(diffs[i].Text))
+			buf.WriteString(diff.Text)
 			buf.Write(codeTagSuffix)
-		case diffs[i].Type == diffmatchpatch.DiffDelete && lineType == DiffLineDel:
+		case diff.Type == diffmatchpatch.DiffDelete && lineType == DiffLineDel:
-			if len(addSpan) > 0 {
-				diffs[i].Text = addSpan + diffs[i].Text
-				addSpan = ""
-			}
-			if strings.HasPrefix(diffs[i].Text, "</span>") {
-				buf.WriteString("</span>")
-				diffs[i].Text = strings.TrimPrefix(diffs[i].Text, "</span>")
-			}
-			m := addSpanRegex.FindStringSubmatchIndex(diffs[i].Text)
-			if m != nil {
-				addSpan = diffs[i].Text[m[0]:m[1]]
-				diffs[i].Text = strings.TrimSuffix(diffs[i].Text, addSpan)
-			}
 			buf.Write(removedCodePrefix)
-			buf.WriteString(getLineContent(diffs[i].Text))
+			buf.WriteString(diff.Text)
 			buf.Write(codeTagSuffix)
 		}
 	}
@@ -333,6 +356,9 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine) tem
 
 	diffRecord := diffMatchPatch.DiffMain(highlight.Code(diffSection.FileName, diff1[1:]), highlight.Code(diffSection.FileName, diff2[1:]), true)
 	diffRecord = diffMatchPatch.DiffCleanupEfficiency(diffRecord)
+
+	diffRecord = diffMatchPatch.DiffCleanupEfficiency(diffRecord)
+
 	return diffToHTML(diffSection.FileName, diffRecord, diffLine.Type)
 }
 
@@ -441,91 +467,263 @@ func (diff *Diff) LoadComments(issue *models.Issue, currentUser *models.User) er
 
 const cmdDiffHead = "diff --git "
 
-// ParsePatch builds a Diff object from a io.Reader and some
+// ParsePatch builds a Diff object from a io.Reader and some parameters.
-// parameters.
-// TODO: move this function to gogits/git-module
 func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*Diff, error) {
-	var (
+	var curFile *DiffFile
-		diff       = &Diff{Files: make([]*DiffFile, 0)}
+
-		curFile    = &DiffFile{}
+	diff := &Diff{Files: make([]*DiffFile, 0)}
-		curSection = &DiffSection{
+
-			Lines: make([]*DiffLine, 0, 10),
+	sb := strings.Builder{}
+
+	// OK let's set a reasonable buffer size.
+	// This should be let's say at least the size of maxLineCharacters or 4096 whichever is larger.
+	readerSize := maxLineCharacters
+	if readerSize < 4096 {
+		readerSize = 4096
+	}
+
+	input := bufio.NewReaderSize(reader, readerSize)
+	line, err := input.ReadString('\n')
+	if err != nil {
+		if err == io.EOF {
+			return diff, nil
+		}
+		return diff, err
+	}
+parsingLoop:
+	for {
+		// 1. A patch file always begins with `diff --git ` + `a/path b/path` (possibly quoted)
+		// if it does not we have bad input!
+		if !strings.HasPrefix(line, cmdDiffHead) {
+			return diff, fmt.Errorf("Invalid first file line: %s", line)
 		}
 
-		leftLine, rightLine int
+		// TODO: Handle skipping first n files
-		lineCount           int
+		if len(diff.Files) >= maxFiles {
-		curFileLinesCount   int
+			diff.IsIncomplete = true
-		curFileLFSPrefix    bool
+			_, err := io.Copy(ioutil.Discard, reader)
+			if err != nil {
+				// By the definition of io.Copy this never returns io.EOF
+				return diff, fmt.Errorf("Copy: %v", err)
+			}
+			break parsingLoop
+		}
+
+		curFile = createDiffFile(diff, line)
+		diff.Files = append(diff.Files, curFile)
+
+		// 2. It is followed by one or more extended header lines:
+		//
+		//     old mode <mode>
+		//     new mode <mode>
+		//     deleted file mode <mode>
+		//     new file mode <mode>
+		//     copy from <path>
+		//     copy to <path>
+		//     rename from <path>
+		//     rename to <path>
+		//     similarity index <number>
+		//     dissimilarity index <number>
+		//     index <hash>..<hash> <mode>
+		//
+		// * <mode> 6-digit octal numbers including the file type and file permission bits.
+		// * <path> does not include the a/ and b/ prefixes
+		// * <number> percentage of unchanged lines for similarity, percentage of changed
+		//   lines dissimilarity as integer rounded down with terminal %. 100% => equal files.
+		// * The index line includes the blob object names before and after the change.
+		//   The <mode> is included if the file mode does not change; otherwise, separate
+		//   lines indicate the old and the new mode.
+		// 3. Following this header the "standard unified" diff format header may be encountered: (but not for every case...)
+		//
+		//     --- a/<path>
+		//     +++ b/<path>
+		//
+		// With multiple hunks
+		//
+		//     @@ <hunk descriptor> @@
+		//     +added line
+		//     -removed line
+		//      unchanged line
+		//
+		// 4. Binary files get:
+		//
+		//     Binary files a/<path> and b/<path> differ
+		//
+		// but one of a/<path> and b/<path> could be /dev/null.
+	curFileLoop:
+		for {
+			line, err = input.ReadString('\n')
+			if err != nil {
+				if err != io.EOF {
+					return diff, err
+				}
+				break parsingLoop
+			}
+			switch {
+			case strings.HasPrefix(line, cmdDiffHead):
+				break curFileLoop
+			case strings.HasPrefix(line, "old mode ") ||
+				strings.HasPrefix(line, "new mode "):
+				if strings.HasSuffix(line, " 160000\n") {
+					curFile.IsSubmodule = true
+				}
+			case strings.HasPrefix(line, "copy from "):
+				curFile.IsRenamed = true
+				curFile.Type = DiffFileCopy
+			case strings.HasPrefix(line, "copy to "):
+				curFile.IsRenamed = true
+				curFile.Type = DiffFileCopy
+			case strings.HasPrefix(line, "new file"):
+				curFile.Type = DiffFileAdd
+				curFile.IsCreated = true
+				if strings.HasSuffix(line, " 160000\n") {
+					curFile.IsSubmodule = true
+				}
+			case strings.HasPrefix(line, "deleted"):
+				curFile.Type = DiffFileDel
+				curFile.IsDeleted = true
+				if strings.HasSuffix(line, " 160000\n") {
+					curFile.IsSubmodule = true
+				}
+			case strings.HasPrefix(line, "index"):
+				if strings.HasSuffix(line, " 160000\n") {
+					curFile.IsSubmodule = true
+				}
+			case strings.HasPrefix(line, "similarity index 100%"):
+				curFile.Type = DiffFileRename
+			case strings.HasPrefix(line, "Binary"):
+				curFile.IsBin = true
+			case strings.HasPrefix(line, "--- "):
+				// Do nothing with this line
+			case strings.HasPrefix(line, "+++ "):
+				// Do nothing with this line
+				lineBytes, isFragment, err := parseHunks(curFile, maxLines, maxLineCharacters, input)
+				diff.TotalAddition += curFile.Addition
+				diff.TotalDeletion += curFile.Deletion
+				if err != nil {
+					if err != io.EOF {
+						return diff, err
+					}
+					break parsingLoop
+				}
+				sb.Reset()
+				_, _ = sb.Write(lineBytes)
+				for isFragment {
+					lineBytes, isFragment, err = input.ReadLine()
+					if err != nil {
+						// Now by the definition of ReadLine this cannot be io.EOF
+						return diff, fmt.Errorf("Unable to ReadLine: %v", err)
+					}
+					_, _ = sb.Write(lineBytes)
+				}
+				line = sb.String()
+				sb.Reset()
+
+				break curFileLoop
+			}
+		}
+
+	}
+
+	// FIXME: There are numerous issues with this:
+	// - we might want to consider detecting encoding while parsing but...
+	// - we're likely to fail to get the correct encoding here anyway as we won't have enough information
+	// - and this doesn't really account for changes in encoding
+	var buf bytes.Buffer
+	for _, f := range diff.Files {
+		buf.Reset()
+		for _, sec := range f.Sections {
+			for _, l := range sec.Lines {
+				if l.Type == DiffLineSection {
+					continue
+				}
+				buf.WriteString(l.Content[1:])
+				buf.WriteString("\n")
+			}
+		}
+		charsetLabel, err := charset.DetectEncoding(buf.Bytes())
+		if charsetLabel != "UTF-8" && err == nil {
+			encoding, _ := stdcharset.Lookup(charsetLabel)
+			if encoding != nil {
+				d := encoding.NewDecoder()
+				for _, sec := range f.Sections {
+					for _, l := range sec.Lines {
+						if l.Type == DiffLineSection {
+							continue
+						}
+						if c, _, err := transform.String(d, l.Content[1:]); err == nil {
+							l.Content = l.Content[0:1] + c
+						}
+					}
+				}
+			}
+		}
+	}
+
+	diff.NumFiles = len(diff.Files)
+	return diff, nil
+}
+
+func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio.Reader) (lineBytes []byte, isFragment bool, err error) {
+	sb := strings.Builder{}
+
+	var (
+		curSection        *DiffSection
+		curFileLinesCount int
+		curFileLFSPrefix  bool
 	)
 
-	input := bufio.NewReader(reader)
+	leftLine, rightLine := 1, 1
-	isEOF := false
-	for !isEOF {
-		var linebuf bytes.Buffer
-		for {
-			b, err := input.ReadByte()
-			if err != nil {
-				if err == io.EOF {
-					isEOF = true
-					break
-				} else {
-					return nil, fmt.Errorf("ReadByte: %v", err)
-				}
-			}
-			if b == '\n' {
-				break
-			}
-			if linebuf.Len() < maxLineCharacters {
-				linebuf.WriteByte(b)
-			} else if linebuf.Len() == maxLineCharacters {
-				curFile.IsIncomplete = true
-			}
-		}
-		line := linebuf.String()
 
-		if strings.HasPrefix(line, "+++ ") || strings.HasPrefix(line, "--- ") || len(line) == 0 {
+	for {
-			continue
+		for isFragment {
-		}
-
-		trimLine := strings.Trim(line, "+- ")
-
-		if trimLine == models.LFSMetaFileIdentifier {
-			curFileLFSPrefix = true
-		}
-
-		if curFileLFSPrefix && strings.HasPrefix(trimLine, models.LFSMetaFileOidPrefix) {
-			oid := strings.TrimPrefix(trimLine, models.LFSMetaFileOidPrefix)
-
-			if len(oid) == 64 {
-				m := &models.LFSMetaObject{Oid: oid}
-				count, err := models.Count(m)
-
-				if err == nil && count > 0 {
-					curFile.IsBin = true
-					curFile.IsLFSFile = true
-					curSection.Lines = nil
-				}
-			}
-		}
-
-		curFileLinesCount++
-		lineCount++
-
-		// Diff data too large, we only show the first about maxLines lines
-		if curFileLinesCount >= maxLines {
 			curFile.IsIncomplete = true
+			_, isFragment, err = input.ReadLine()
+			if err != nil {
+				// Now by the definition of ReadLine this cannot be io.EOF
+				err = fmt.Errorf("Unable to ReadLine: %v", err)
+				return
+			}
 		}
-		switch {
+		sb.Reset()
-		case line[0] == ' ':
+		lineBytes, isFragment, err = input.ReadLine()
-			diffLine := &DiffLine{Type: DiffLinePlain, Content: line, LeftIdx: leftLine, RightIdx: rightLine}
+		if err != nil {
-			leftLine++
+			if err == io.EOF {
-			rightLine++
+				return
-			curSection.Lines = append(curSection.Lines, diffLine)
+			}
-			curSection.FileName = curFile.Name
+			err = fmt.Errorf("Unable to ReadLine: %v", err)
-			continue
+			return
-		case line[0] == '@':
+		}
+		if lineBytes[0] == 'd' {
+			// End of hunks
+			return
+		}
+
+		switch lineBytes[0] {
+		case '@':
+			if curFileLinesCount >= maxLines {
+				curFile.IsIncomplete = true
+				continue
+			}
+
+			_, _ = sb.Write(lineBytes)
+			for isFragment {
+				// This is very odd indeed - we're in a section header and the line is too long
+				// This really shouldn't happen...
+				lineBytes, isFragment, err = input.ReadLine()
+				if err != nil {
+					// Now by the definition of ReadLine this cannot be io.EOF
+					err = fmt.Errorf("Unable to ReadLine: %v", err)
+					return
+				}
+				_, _ = sb.Write(lineBytes)
+			}
+			line := sb.String()
+
+			// Create a new section to represent this hunk
 			curSection = &DiffSection{}
 			curFile.Sections = append(curFile.Sections, curSection)
+
 			lineSectionInfo := getDiffLineSectionInfo(curFile.Name, line, leftLine-1, rightLine-1)
 			diffLine := &DiffLine{
 				Type:        DiffLineSection,
@@ -538,148 +736,136 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
 			leftLine = lineSectionInfo.LeftIdx
 			rightLine = lineSectionInfo.RightIdx
 			continue
-		case line[0] == '+':
+		case '\\':
+			if curFileLinesCount >= maxLines {
+				curFile.IsIncomplete = true
+				continue
+			}
+			// This is used only to indicate that the current file does not have a terminal newline
+			if !bytes.Equal(lineBytes, []byte("\\ No newline at end of file")) {
+				err = fmt.Errorf("Unexpected line in hunk: %s", string(lineBytes))
+				return
+			}
+			// Technically this should be the end the file!
+			// FIXME: we should be putting a marker at the end of the file if there is no terminal new line
+			continue
+		case '+':
+			curFileLinesCount++
 			curFile.Addition++
-			diff.TotalAddition++
+			if curFileLinesCount >= maxLines {
-			diffLine := &DiffLine{Type: DiffLineAdd, Content: line, RightIdx: rightLine}
+				curFile.IsIncomplete = true
+				continue
+			}
+			diffLine := &DiffLine{Type: DiffLineAdd, RightIdx: rightLine}
 			rightLine++
 			curSection.Lines = append(curSection.Lines, diffLine)
-			curSection.FileName = curFile.Name
+		case '-':
-			continue
+			curFileLinesCount++
-		case line[0] == '-':
 			curFile.Deletion++
-			diff.TotalDeletion++
+			if curFileLinesCount >= maxLines {
-			diffLine := &DiffLine{Type: DiffLineDel, Content: line, LeftIdx: leftLine}
+				curFile.IsIncomplete = true
+				continue
+			}
+			diffLine := &DiffLine{Type: DiffLineDel, LeftIdx: leftLine}
 			if leftLine > 0 {
 				leftLine++
 			}
 			curSection.Lines = append(curSection.Lines, diffLine)
-			curSection.FileName = curFile.Name
+		case ' ':
-		case strings.HasPrefix(line, "Binary"):
+			curFileLinesCount++
-			curFile.IsBin = true
+			if curFileLinesCount >= maxLines {
-			continue
+				curFile.IsIncomplete = true
+				continue
+			}
+			diffLine := &DiffLine{Type: DiffLinePlain, LeftIdx: leftLine, RightIdx: rightLine}
+			leftLine++
+			rightLine++
+			curSection.Lines = append(curSection.Lines, diffLine)
+		default:
+			// This is unexpected
+			err = fmt.Errorf("Unexpected line in hunk: %s", string(lineBytes))
+			return
 		}
 
-		// Get new file.
+		line := string(lineBytes)
-		if strings.HasPrefix(line, cmdDiffHead) {
+		if isFragment {
-			if len(diff.Files) >= maxFiles {
+			curFile.IsIncomplete = true
-				diff.IsIncomplete = true
+			for isFragment {
-				_, err := io.Copy(ioutil.Discard, reader)
+				lineBytes, isFragment, err = input.ReadLine()
 				if err != nil {
-					return nil, fmt.Errorf("Copy: %v", err)
+					// Now by the definition of ReadLine this cannot be io.EOF
+					err = fmt.Errorf("Unable to ReadLine: %v", err)
+					return
 				}
-				break
 			}
+		}
+		if len(line) > maxLineCharacters {
+			curFile.IsIncomplete = true
+			line = line[:maxLineCharacters]
+		}
+		curSection.Lines[len(curSection.Lines)-1].Content = line
 
-			// Note: In case file name is surrounded by double quotes (it happens only in git-shell).
+		// handle LFS
-			// e.g. diff --git "a/xxx" "b/xxx"
+		if line[1:] == models.LFSMetaFileIdentifier {
-			var a string
+			curFileLFSPrefix = true
-			var b string
+		} else if curFileLFSPrefix && strings.HasPrefix(line[1:], models.LFSMetaFileOidPrefix) {
+			oid := strings.TrimPrefix(line[1:], models.LFSMetaFileOidPrefix)
+			if len(oid) == 64 {
+				m := &models.LFSMetaObject{Oid: oid}
+				count, err := models.Count(m)
 
-			rd := strings.NewReader(line[len(cmdDiffHead):])
+				if err == nil && count > 0 {
-			char, _ := rd.ReadByte()
+					curFile.IsBin = true
-			_ = rd.UnreadByte()
+					curFile.IsLFSFile = true
-			if char == '"' {
+					curSection.Lines = nil
-				fmt.Fscanf(rd, "%q ", &a)
-				if a[0] == '\\' {
-					a = a[1:]
-				}
-			} else {
-				fmt.Fscanf(rd, "%s ", &a)
-			}
-			char, _ = rd.ReadByte()
-			_ = rd.UnreadByte()
-			if char == '"' {
-				fmt.Fscanf(rd, "%q", &b)
-				if b[0] == '\\' {
-					b = b[1:]
-				}
-			} else {
-				fmt.Fscanf(rd, "%s", &b)
-			}
-			a = a[2:]
-			b = b[2:]
-
-			curFile = &DiffFile{
-				Name:      b,
-				OldName:   a,
-				Index:     len(diff.Files) + 1,
-				Type:      DiffFileChange,
-				Sections:  make([]*DiffSection, 0, 10),
-				IsRenamed: a != b,
-			}
-			diff.Files = append(diff.Files, curFile)
-			curFileLinesCount = 0
-			leftLine = 1
-			rightLine = 1
-			curFileLFSPrefix = false
-
-			// Check file diff type and is submodule.
-			for {
-				line, err := input.ReadString('\n')
-				if err != nil {
-					if err == io.EOF {
-						isEOF = true
-					} else {
-						return nil, fmt.Errorf("ReadString: %v", err)
-					}
-				}
-
-				switch {
-				case strings.HasPrefix(line, "copy from "):
-					curFile.IsRenamed = true
-					curFile.Type = DiffFileCopy
-				case strings.HasPrefix(line, "copy to "):
-					curFile.IsRenamed = true
-					curFile.Type = DiffFileCopy
-				case strings.HasPrefix(line, "new file"):
-					curFile.Type = DiffFileAdd
-					curFile.IsCreated = true
-				case strings.HasPrefix(line, "deleted"):
-					curFile.Type = DiffFileDel
-					curFile.IsDeleted = true
-				case strings.HasPrefix(line, "index"):
-					curFile.Type = DiffFileChange
-				case strings.HasPrefix(line, "similarity index 100%"):
-					curFile.Type = DiffFileRename
-				}
-				if curFile.Type > 0 {
-					if strings.HasSuffix(line, " 160000\n") {
-						curFile.IsSubmodule = true
-					}
-					break
 				}
 			}
 		}
 	}
+}
 
-	// FIXME: detect encoding while parsing.
+func createDiffFile(diff *Diff, line string) *DiffFile {
-	var buf bytes.Buffer
+	// The a/ and b/ filenames are the same unless rename/copy is involved.
-	for _, f := range diff.Files {
+	// Especially, even for a creation or a deletion, /dev/null is not used
-		buf.Reset()
+	// in place of the a/ or b/ filenames.
-		for _, sec := range f.Sections {
+	//
-			for _, l := range sec.Lines {
+	// When rename/copy is involved, file1 and file2 show the name of the
-				buf.WriteString(l.Content)
+	// source file of the rename/copy and the name of the file that rename/copy
-				buf.WriteString("\n")
+	// produces, respectively.
-			}
+	//
-		}
+	// Path names are quoted if necessary.
-		charsetLabel, err := charset.DetectEncoding(buf.Bytes())
+	//
-		if charsetLabel != "UTF-8" && err == nil {
+	// This means that you should always be able to determine the file name even when there
-			encoding, _ := stdcharset.Lookup(charsetLabel)
+	// there is potential ambiguity...
-			if encoding != nil {
+	//
-				d := encoding.NewDecoder()
+	// but we can be simpler with our heuristics by just forcing git to prefix things nicely
-				for _, sec := range f.Sections {
+	curFile := &DiffFile{
-					for _, l := range sec.Lines {
+		Index:    len(diff.Files) + 1,
-						if c, _, err := transform.String(d, l.Content); err == nil {
+		Type:     DiffFileChange,
-							l.Content = c
+		Sections: make([]*DiffSection, 0, 10),
-						}
-					}
-				}
-			}
-		}
 	}
-	diff.NumFiles = len(diff.Files)
+
-	return diff, nil
+	rd := strings.NewReader(line[len(cmdDiffHead):] + " ")
+	curFile.Type = DiffFileChange
+	curFile.OldName = readFileName(rd)
+	curFile.Name = readFileName(rd)
+	curFile.IsRenamed = curFile.Name != curFile.OldName
+	return curFile
+}
+
+func readFileName(rd *strings.Reader) string {
+	var name string
+	char, _ := rd.ReadByte()
+	_ = rd.UnreadByte()
+	if char == '"' {
+		fmt.Fscanf(rd, "%q ", &name)
+		if name[0] == '\\' {
+			name = name[1:]
+		}
+	} else {
+		fmt.Fscanf(rd, "%s ", &name)
+	}
+	return name[2:]
 }
 
 // GetDiffRange builds a Diff between two commits of a repository.
@@ -709,7 +895,14 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID
 	defer cancel()
 	var cmd *exec.Cmd
 	if (len(beforeCommitID) == 0 || beforeCommitID == git.EmptySHA) && commit.ParentCount() == 0 {
-		cmd = exec.CommandContext(ctx, git.GitExecutable, "show", afterCommitID)
+		diffArgs := []string{"diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M"}
+		if len(whitespaceBehavior) != 0 {
+			diffArgs = append(diffArgs, whitespaceBehavior)
+		}
+		// append empty tree ref
+		diffArgs = append(diffArgs, "4b825dc642cb6eb9a060e54bf8d69288fbee4904")
+		diffArgs = append(diffArgs, afterCommitID)
+		cmd = exec.CommandContext(ctx, git.GitExecutable, diffArgs...)
 	} else {
 		actualBeforeCommitID := beforeCommitID
 		if len(actualBeforeCommitID) == 0 {

services/gitdiff/gitdiff_test.go

@@ -9,6 +9,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"html/template"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -50,7 +51,7 @@ func TestDiffToHTML(t *testing.T) {
 		{Type: dmp.DiffEqual, Text: "</span> <span class=\"p\">{</span>"},
 	}, DiffLineAdd))
 
-	assertEqual(t, "<span class=\"nx\">tagURL</span> <span class=\"o\">:=</span> <span class=\"removed-code\"><span class=\"nx\">fmt</span><span class=\"p\">.</span><span class=\"nf\">Sprintf</span><span class=\"p\">(</span><span class=\"s\">&#34;## [%s](%s/%s/%s/%s?q=&amp;type=all&amp;state=closed&amp;milestone=%d) - %s&#34;</span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">Milestone\"</span></span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">BaseURL</span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">Owner</span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">Repo</span><span class=\"p\">,</span> <span class=\"nx\"><span class=\"removed-code\">from</span><span class=\"p\">,</span> <span class=\"nx\">milestoneID</span><span class=\"p\">,</span> <span class=\"nx\">time</span><span class=\"p\">.</span><span class=\"nf\">Now</span><span class=\"p\">(</span><span class=\"p\">)</span><span class=\"p\">.</span><span class=\"nf\">Format</span><span class=\"p\">(</span><span class=\"s\">&#34;2006-01-02&#34;</span><span class=\"p\">)</span></span><span class=\"p\">)</span>", diffToHTML("", []dmp.Diff{
+	assertEqual(t, "<span class=\"nx\">tagURL</span> <span class=\"o\">:=</span> <span class=\"removed-code\"><span class=\"nx\">fmt</span><span class=\"p\">.</span><span class=\"nf\">Sprintf</span><span class=\"p\">(</span><span class=\"s\">&#34;## [%s](%s/%s/%s/%s?q=&amp;type=all&amp;state=closed&amp;milestone=%d) - %s&#34;</span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">Milestone\"</span></span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">BaseURL</span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">Owner</span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">Repo</span><span class=\"p\">,</span> <span class=\"removed-code\"><span class=\"nx\">from</span><span class=\"p\">,</span> <span class=\"nx\">milestoneID</span><span class=\"p\">,</span> <span class=\"nx\">time</span><span class=\"p\">.</span><span class=\"nf\">Now</span><span class=\"p\">(</span><span class=\"p\">)</span><span class=\"p\">.</span><span class=\"nf\">Format</span><span class=\"p\">(</span><span class=\"s\">&#34;2006-01-02&#34;</span><span class=\"p\">)</span></span><span class=\"p\">)</span>", diffToHTML("", []dmp.Diff{
 		{Type: dmp.DiffEqual, Text: "<span class=\"nx\">tagURL</span> <span class=\"o\">:=</span> <span class=\"n"},
 		{Type: dmp.DiffDelete, Text: "x\">fmt</span><span class=\"p\">.</span><span class=\"nf\">Sprintf</span><span class=\"p\">(</span><span class=\"s\">&#34;## [%s](%s/%s/%s/%s?q=&amp;type=all&amp;state=closed&amp;milestone=%d) - %s&#34;</span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">Milestone\""},
 		{Type: dmp.DiffInsert, Text: "f\">getGiteaTagURL</span><span class=\"p\">(</span><span class=\"nx\">client"},
@@ -60,7 +61,7 @@ func TestDiffToHTML(t *testing.T) {
 		{Type: dmp.DiffEqual, Text: "</span><span class=\"p\">)</span>"},
 	}, DiffLineDel))
 
-	assertEqual(t, "<span class=\"nx\">r</span><span class=\"p\">.</span><span class=\"nf\">WrapperRenderer</span><span class=\"p\">(</span><span class=\"nx\">w</span><span class=\"p\">,</span> <span class=\"nx\"><span class=\"removed-code\">language</span></span><span class=\"removed-code\"><span class=\"p\">,</span> <span class=\"kc\">true</span><span class=\"p\">,</span> <span class=\"nx\">attrs</span></span><span class=\"p\">,</span> <span class=\"kc\">false</span><span class=\"p\">)</span>", diffToHTML("", []dmp.Diff{
+	assertEqual(t, "<span class=\"nx\">r</span><span class=\"p\">.</span><span class=\"nf\">WrapperRenderer</span><span class=\"p\">(</span><span class=\"nx\">w</span><span class=\"p\">,</span> <span class=\"removed-code\"><span class=\"nx\">language</span></span><span class=\"removed-code\"><span class=\"p\">,</span> <span class=\"kc\">true</span><span class=\"p\">,</span> <span class=\"nx\">attrs</span></span><span class=\"p\">,</span> <span class=\"kc\">false</span><span class=\"p\">)</span>", diffToHTML("", []dmp.Diff{
 		{Type: dmp.DiffEqual, Text: "<span class=\"nx\">r</span><span class=\"p\">.</span><span class=\"nf\">WrapperRenderer</span><span class=\"p\">(</span><span class=\"nx\">w</span><span class=\"p\">,</span> <span class=\"nx\">"},
 		{Type: dmp.DiffDelete, Text: "language</span><span "},
 		{Type: dmp.DiffEqual, Text: "c"},
@@ -74,6 +75,30 @@ func TestDiffToHTML(t *testing.T) {
 		{Type: dmp.DiffInsert, Text: "lass=\"p\">,</span> <span class=\"kc\">true</span><span class=\"p\">,</span> <span class=\"nx\">attrs"},
 		{Type: dmp.DiffEqual, Text: "</span><span class=\"p\">,</span> <span class=\"kc\">false</span><span class=\"p\">)</span>"},
 	}, DiffLineAdd))
+
+	assertEqual(t, "<span class=\"k\">print</span><span class=\"added-code\"></span><span class=\"added-code\"><span class=\"p\">(</span></span><span class=\"sa\"></span><span class=\"s2\">&#34;</span><span class=\"s2\">// </span><span class=\"s2\">&#34;</span><span class=\"p\">,</span> <span class=\"n\">sys</span><span class=\"o\">.</span><span class=\"n\">argv</span><span class=\"added-code\"><span class=\"p\">)</span></span>", diffToHTML("", []dmp.Diff{
+		{Type: dmp.DiffEqual, Text: "<span class=\"k\">print</span>"},
+		{Type: dmp.DiffInsert, Text: "<span"},
+		{Type: dmp.DiffEqual, Text: " "},
+		{Type: dmp.DiffInsert, Text: "class=\"p\">(</span>"},
+		{Type: dmp.DiffEqual, Text: "<span class=\"sa\"></span><span class=\"s2\">&#34;</span><span class=\"s2\">// </span><span class=\"s2\">&#34;</span><span class=\"p\">,</span> <span class=\"n\">sys</span><span class=\"o\">.</span><span class=\"n\">argv</span>"},
+		{Type: dmp.DiffInsert, Text: "<span class=\"p\">)</span>"},
+	}, DiffLineAdd))
+
+	assertEqual(t, "sh <span class=\"added-code\">&#39;useradd -u $(stat -c &#34;%u&#34; .gitignore) jenkins</span>&#39;", diffToHTML("", []dmp.Diff{
+		{Type: dmp.DiffEqual, Text: "sh &#3"},
+		{Type: dmp.DiffDelete, Text: "4;useradd -u 111 jenkins&#34"},
+		{Type: dmp.DiffInsert, Text: "9;useradd -u $(stat -c &#34;%u&#34; .gitignore) jenkins&#39"},
+		{Type: dmp.DiffEqual, Text: ";"},
+	}, DiffLineAdd))
+
+	assertEqual(t, "<span class=\"x\">							&lt;h<span class=\"added-code\">4 class=</span><span class=\"added-code\">&#34;release-list-title df ac&#34;</span>&gt;</span>", diffToHTML("", []dmp.Diff{
+		{Type: dmp.DiffEqual, Text: "<span class=\"x\">							&lt;h"},
+		{Type: dmp.DiffInsert, Text: "4 class=&#"},
+		{Type: dmp.DiffEqual, Text: "3"},
+		{Type: dmp.DiffInsert, Text: "4;release-list-title df ac&#34;"},
+		{Type: dmp.DiffEqual, Text: "&gt;</span>"},
+	}, DiffLineAdd))
 }
 
 func TestParsePatch_singlefile(t *testing.T) {
@@ -129,11 +154,11 @@ func TestParsePatch_singlefile(t *testing.T) {
 			name: "really weird filename",
 			gitdiff: `diff --git "\\a/a b/file b/a a/file" "\\b/a b/file b/a a/file"
 index d2186f1..f5c8ed2 100644
---- "\\a/a b/file b/a a/file"	
+--- "\\a/a b/file b/a a/file"	` + `
-+++ "\\b/a b/file b/a a/file"	
++++ "\\b/a b/file b/a a/file"	` + `
 @@ -1,3 +1,2 @@
  Create a weird file.
- 
+ ` + `
 -and what does diff do here?
 \ No newline at end of file`,
 			addition:    0,
@@ -146,7 +171,7 @@ index d2186f1..f5c8ed2 100644
 			gitdiff: `diff --git "\\a/file with blanks" "\\b/file with blanks"
 deleted file mode 100644
 index 898651a..0000000
---- "\\a/file with blanks"	
+--- "\\a/file with blanks" ` + `
 +++ /dev/null
 @@ -1,5 +0,0 @@
 -a blank file
@@ -182,6 +207,27 @@ rename to a b/a a/file b/b file
 			oldFilename: "a b/file b/a a/file",
 			filename:    "a b/a a/file b/b file",
 		},
+		{
+			name: "minuses-and-pluses",
+			gitdiff: `diff --git a/minuses-and-pluses b/minuses-and-pluses
+index 6961180..9ba1a00 100644
+--- a/minuses-and-pluses
++++ b/minuses-and-pluses
+@@ -1,4 +1,4 @@
+--- 1st line
+-++ 2nd line
+--- 3rd line
+-++ 4th line
++++ 1st line
++-- 2nd line
++++ 3rd line
++-- 4th line
+`,
+			oldFilename: "minuses-and-pluses",
+			filename:    "minuses-and-pluses",
+			addition:    4,
+			deletion:    4,
+		},
 	}
 
 	for _, testcase := range tests {
@@ -218,7 +264,83 @@ rename to a b/a a/file b/b file
 		})
 	}
 
-	var diff = `diff --git "a/README.md" "b/README.md"
+	// Test max lines
+	diffBuilder := &strings.Builder{}
+
+	var diff = `diff --git a/newfile2 b/newfile2
+new file mode 100644
+index 0000000..6bb8f39
+--- /dev/null
++++ b/newfile2
+@@ -0,0 +1,35 @@
+`
+	diffBuilder.WriteString(diff)
+
+	for i := 0; i < 35; i++ {
+		diffBuilder.WriteString("+line" + strconv.Itoa(i) + "\n")
+	}
+	diff = diffBuilder.String()
+	result, err := ParsePatch(20, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff))
+	if err != nil {
+		t.Errorf("There should not be an error: %v", err)
+	}
+	if !result.Files[0].IsIncomplete {
+		t.Errorf("Files should be incomplete! %v", result.Files[0])
+	}
+	result, err = ParsePatch(40, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff))
+	if err != nil {
+		t.Errorf("There should not be an error: %v", err)
+	}
+	if result.Files[0].IsIncomplete {
+		t.Errorf("Files should not be incomplete! %v", result.Files[0])
+	}
+	result, err = ParsePatch(40, 5, setting.Git.MaxGitDiffFiles, strings.NewReader(diff))
+	if err != nil {
+		t.Errorf("There should not be an error: %v", err)
+	}
+	if !result.Files[0].IsIncomplete {
+		t.Errorf("Files should be incomplete! %v", result.Files[0])
+	}
+
+	// Test max characters
+	diff = `diff --git a/newfile2 b/newfile2
+new file mode 100644
+index 0000000..6bb8f39
+--- /dev/null
++++ b/newfile2
+@@ -0,0 +1,35 @@
+`
+	diffBuilder.Reset()
+	diffBuilder.WriteString(diff)
+
+	for i := 0; i < 33; i++ {
+		diffBuilder.WriteString("+line" + strconv.Itoa(i) + "\n")
+	}
+	diffBuilder.WriteString("+line33")
+	for i := 0; i < 512; i++ {
+		diffBuilder.WriteString("0123456789ABCDEF")
+	}
+	diffBuilder.WriteByte('\n')
+	diffBuilder.WriteString("+line" + strconv.Itoa(34) + "\n")
+	diffBuilder.WriteString("+line" + strconv.Itoa(35) + "\n")
+	diff = diffBuilder.String()
+
+	result, err = ParsePatch(20, 4096, setting.Git.MaxGitDiffFiles, strings.NewReader(diff))
+	if err != nil {
+		t.Errorf("There should not be an error: %v", err)
+	}
+	if !result.Files[0].IsIncomplete {
+		t.Errorf("Files should be incomplete! %v", result.Files[0])
+	}
+	result, err = ParsePatch(40, 4096, setting.Git.MaxGitDiffFiles, strings.NewReader(diff))
+	if err != nil {
+		t.Errorf("There should not be an error: %v", err)
+	}
+	if !result.Files[0].IsIncomplete {
+		t.Errorf("Files should be incomplete! %v", result.Files[0])
+	}
+
+	diff = `diff --git "a/README.md" "b/README.md"
 --- a/README.md
 +++ b/README.md
 @@ -1,3 +1,6 @@
@@ -229,7 +351,7 @@ rename to a b/a a/file b/b file
  Docker Pulls
 + cut off
 + cut off`
-	result, err := ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff))
+	result, err = ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff))
 	if err != nil {
 		t.Errorf("ParsePatch failed: %s", err)
 	}

services/pull/pull.go

@@ -609,7 +609,7 @@ func GetCommitMessages(pr *models.PullRequest) string {
 				}
 				element = element.Next()
 			}
-
+			skip += limit
 		}
 	}
 

services/pull/review.go

@@ -122,41 +122,76 @@ func createCodeComment(doer *models.User, repo *models.Repository, issue *models
 	}
 	defer gitRepo.Close()
 
-	// FIXME validate treePath
+	invalidated := false
-	// Get latest commit referencing the commented line
+	head := pr.GetGitRefName()
-	// No need for get commit for base branch changes
 	if line > 0 {
-		commit, err := gitRepo.LineBlame(pr.GetGitRefName(), gitRepo.Path, treePath, uint(line))
+		if reviewID != 0 {
-		if err == nil {
+			first, err := models.FindComments(models.FindCommentsOptions{
-			commitID = commit.ID.String()
+				ReviewID: reviewID,
-		} else if !(strings.Contains(err.Error(), "exit status 128 - fatal: no such path") || notEnoughLines.MatchString(err.Error())) {
+				Line:     line,
-			return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err)
+				TreePath: treePath,
+				Type:     models.CommentTypeCode,
+				ListOptions: models.ListOptions{
+					PageSize: 1,
+					Page:     1,
+				},
+			})
+			if err == nil && len(first) > 0 {
+				commitID = first[0].CommitSHA
+				invalidated = first[0].Invalidated
+				patch = first[0].Patch
+			} else if err != nil && !models.IsErrCommentNotExist(err) {
+				return nil, fmt.Errorf("Find first comment for %d line %d path %s. Error: %v", reviewID, line, treePath, err)
+			} else {
+				review, err := models.GetReviewByID(reviewID)
+				if err == nil && len(review.CommitID) > 0 {
+					head = review.CommitID
+				} else if err != nil && !models.IsErrReviewNotExist(err) {
+					return nil, fmt.Errorf("GetReviewByID %d. Error: %v", reviewID, err)
+				}
+			}
+		}
+
+		if len(commitID) == 0 {
+			// FIXME validate treePath
+			// Get latest commit referencing the commented line
+			// No need for get commit for base branch changes
+			commit, err := gitRepo.LineBlame(head, gitRepo.Path, treePath, uint(line))
+			if err == nil {
+				commitID = commit.ID.String()
+			} else if !(strings.Contains(err.Error(), "exit status 128 - fatal: no such path") || notEnoughLines.MatchString(err.Error())) {
+				return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err)
+			}
 		}
 	}
 
 	// Only fetch diff if comment is review comment
-	if reviewID != 0 {
+	if len(patch) == 0 && reviewID != 0 {
-		headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName())
+		if len(commitID) == 0 {
-		if err != nil {
+			commitID, err = gitRepo.GetRefCommitID(pr.GetGitRefName())
-			return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err)
+			if err != nil {
+				return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err)
+			}
 		}
+
 		patchBuf := new(bytes.Buffer)
-		if err := git.GetRepoRawDiffForFile(gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, treePath, patchBuf); err != nil {
+		if err := git.GetRepoRawDiffForFile(gitRepo, pr.MergeBase, commitID, git.RawDiffNormal, treePath, patchBuf); err != nil {
-			return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", err, gitRepo.Path, pr.MergeBase, headCommitID, treePath)
+			return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", gitRepo.Path, pr.MergeBase, commitID, treePath, err)
 		}
 		patch = git.CutDiffAroundLine(patchBuf, int64((&models.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines)
 	}
 	return models.CreateComment(&models.CreateCommentOptions{
-		Type:      models.CommentTypeCode,
+		Type:        models.CommentTypeCode,
-		Doer:      doer,
+		Doer:        doer,
-		Repo:      repo,
+		Repo:        repo,
-		Issue:     issue,
+		Issue:       issue,
-		Content:   content,
+		Content:     content,
-		LineNum:   line,
+		LineNum:     line,
-		TreePath:  treePath,
+		TreePath:    treePath,
-		CommitSHA: commitID,
+		CommitSHA:   commitID,
-		ReviewID:  reviewID,
+		ReviewID:    reviewID,
-		Patch:     patch,
+		Patch:       patch,
+		Invalidated: invalidated,
 	})
 }
 

snap/snapcraft.yaml

@@ -19,6 +19,8 @@ architectures:
 environment:
   GITEA_CUSTOM: "$SNAP_COMMON"
   GITEA_WORK_DIR: "$SNAP_DATA"
+  GIT_TEMPLATE_DIR: "$SNAP/usr/share/git-core/templates"
+  GIT_EXEC_PATH: "$SNAP/usr/lib/git-core"
 
 apps:
   gitea:

templates/base/alert.tmpl

@@ -1,15 +1,15 @@
 {{if .Flash.ErrorMsg}}
-	<div class="ui negative message">
+	<div class="ui negative message flash-error">
 		<p>{{.Flash.ErrorMsg | Str2html}}</p>
 	</div>
 {{end}}
 {{if .Flash.SuccessMsg}}
-	<div class="ui positive message">
+	<div class="ui positive message flash-success">
 		<p>{{.Flash.SuccessMsg | Str2html}}</p>
 	</div>
 {{end}}
 {{if .Flash.InfoMsg}}
-	<div class="ui info message">
+	<div class="ui info message flash-info">
 		<p>{{.Flash.InfoMsg | Str2html}}</p>
 	</div>
 {{end}}

templates/base/alert_details.tmpl

@@ -0,0 +1,7 @@
+{{.Message}}
+<details>
+    <summary>{{.Summary}}</summary>
+    <code>
+        {{.Details | Str2html}}
+    </code>
+</details>