Backport #22942
This PR refactors and improves the password hashing code within gitea
and makes it possible for server administrators to set the password
hashing parameters
In addition it takes the opportunity to adjust the settings for `pbkdf2`
in order to make the hashing a little stronger.
The majority of this work was inspired by PR #14751 and I would like to
thank @boppy for their work on this.
Thanks to @gusted for the suggestion to adjust the `pbkdf2` hashing
parameters.
Close#14751
---------
Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Backport #22826
Creating a new buffered reader for every part of the blame can miss
lines, as it will read and buffer bytes that the next buffered reader
will not get.
---------
Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Brecht Van Lommel <brecht@blender.org>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Backport #22633
Our trace logging is far from perfect and is difficult to follow.
This PR:
* Add trace logging for process manager add and remove.
* Fixes an errant read file for git refs in getMergeCommit
* Brings in the pullrequest `String` and `ColorFormat` methods
introduced in #22568
* Adds a lot more logging in to testPR etc.
Ref #22578
---------
Signed-off-by: Andrew Thornton <art27@cantab.net>
Backport #22814.
The use of `sort.Search` is wrong: The slice should be sorted, and
`return >= 0` doen't mean it exists, see the
[manual](https://pkg.go.dev/sort#Search).
Could be fixed like this if we really need it:
```diff
diff --git a/modules/charset/escape_stream.go b/modules/charset/escape_stream.go
index 823b63513..fcf1ffbc1 100644
--- a/modules/charset/escape_stream.go
+++ b/modules/charset/escape_stream.go
@@ -20,6 +20,9 @@ import (
var defaultWordRegexp = regexp.MustCompile(`(-?\d*\.\d\w*)|([^\` + "`" + `\~\!\@\#\$\%\^\&\*\(\)\-\=\+\[\{\]\}\\\|\;\:\'\"\,\.\<\>\/\?\s\x00-\x1f]+)`)
func NewEscapeStreamer(locale translation.Locale, next HTMLStreamer, allowed ...rune) HTMLStreamer {
+ sort.Slice(allowed, func(i, j int) bool {
+ return allowed[i] < allowed[j]
+ })
return &escapeStreamer{
escaped: &EscapeStatus{},
PassthroughHTMLStreamer: *NewPassthroughStreamer(next),
@@ -284,14 +287,8 @@ func (e *escapeStreamer) runeTypes(runes ...rune) (types []runeType, confusables
}
func (e *escapeStreamer) isAllowed(r rune) bool {
- if len(e.allowed) == 0 {
- return false
- }
- if len(e.allowed) == 1 {
- return e.allowed[0] == r
- }
-
- return sort.Search(len(e.allowed), func(i int) bool {
+ i := sort.Search(len(e.allowed), func(i int) bool {
return e.allowed[i] >= r
- }) >= 0
+ })
+ return i < len(e.allowed) && e.allowed[i] == r
}
```
But I don't think so, a map is better to do it.
With the upgrade to go 1.20 golangci-lint no longer correctly works. We must therefore upgrade to the latest golangci-lint.
Co-authored-by: techknowlogick <techknowlogick@gitea.io>
Backport #22611
The code for checking if a commit has caused a change in a PR is
extremely inefficient and affects the head repository instead of using a
temporary repository.
This PR therefore makes several significant improvements:
* A temporary repo like that used in merging.
* The diff code is then significant improved to use a three-way diff
instead of comparing diffs (possibly binary) line-by-line - in memory...
Ref #22578
Signed-off-by: Andrew Thornton <art27@cantab.net>
Backport #22627
Adding the related comment to the issue and pull request status change
in the UI notifications allows to navigate directly to the specific
event in its dedicated view, easing the reading of last comments and to
the editor for additional comments if desired.
Co-authored-by: Felipe Leopoldo Sologuren Gutiérrez <fsologureng@users.noreply.github.com>
Backport #22392Fix#22386
`GetDirectorySize` moved as `getDirectorySize` because it becomes a
special function which should not be put in `util`.
Co-authored-by: Jason Song <i@wolfogre.com>
Backport #22146
There are a few places in FlushQueueWithContext which make an incorrect
assumption about how `select` on multiple channels works.
The problem is best expressed by looking at the following example:
```go
package main
import "fmt"
func main() {
closedChan := make(chan struct{})
close(closedChan)
toClose := make(chan struct{})
count := 0
for {
select {
case <-closedChan:
count++
fmt.Println(count)
if count == 2 {
close(toClose)
}
case <-toClose:
return
}
}
}
```
This PR double-checks that the contexts are closed outside of checking
if there is data in the dataChan. It also rationalises the WorkerPool
FlushWithContext because the previous implementation failed to handle
pausing correctly. This will probably fix the underlying problem in
#22145Fix#22145
Signed-off-by: Andrew Thornton <art27@cantab.net>
backport #22261
A drawback is the previous generated template has been cached, so you
cannot get error in the UI but only from log
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
Co-authored-by: delvh <dev.lh@web.de>
backport #22246
Since #20218 introduced internal watching template, template watching
should be removed from `air`. This will prevent restart the whole server
once the template files changed to speed up developing when using `make
watch`.
To ensure `make watch` will reuse template watching, this PR introduced
a new ENV `GITEA_RUN_MODE` to make sure `make watch` will always run in
a dev mode of Gitea so that template watching will open.
This PR also added more exclude testdata directories.
Co-authored-by: 6543 <6543@obermui.de>
backport #22119
This PR changed the Auth interface signature from `Verify(http
*http.Request, w http.ResponseWriter, store DataStore, sess
SessionStore) *user_model.User`
to
`Verify(http *http.Request, w http.ResponseWriter, store DataStore, sess
SessionStore) (*user_model.User, error)`.
There is a new return argument `error` which means the verification
condition matched but verify process failed, we should stop the auth
process.
Before this PR, when return a `nil` user, we don't know the reason why
it returned `nil`. If the match condition is not satisfied or it
verified failure? For these two different results, we should have
different handler. If the match condition is not satisfied, we should
try next auth method and if there is no more auth method, it's an
anonymous user. If the condition matched but verify failed, the auth
process should be stop and return immediately.
This will fix#20563
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
Co-authored-by: Jason Song <i@wolfogre.com>
Backport #22130
For a long time Gitea has tested PR patches using a git apply --check
method, and in fact prior to the introduction of a read-tree assisted
three-way merge in #18004, this was the only way of checking patches.
Since #18004, the git apply --check method has been a fallback method,
only used when the read-tree three-way merge method has detected a
conflict. The read-tree assisted three-way merge method is much faster
and less resource intensive method of detecting conflicts. #18004 kept
the git apply method around because it was thought possible that this
fallback might be able to rectify conflicts that the read-tree three-way
merge detected. I am not certain if this could ever be the case.
Given the uncertainty here and the now relative stability of the
read-tree method - this PR makes using this fallback optional but
enables it by default. A `log.Critical` has been added which will alert
if the `git apply --check` method was successful at checking a PR that
`read-tree` failed on.
The hope is that none of these log.Critical messages will be found and
there will be no significant difference in conflict detection. Thus we
will be able to remove the git apply fallback in future, and/or improve
the read-tree three-way merge method to catch any conflicts that git
apply method might have been able to fix.
An additional benefit for anyone who disables the check method is that
patch checking should be significantly less resource intensive and much
quicker.
(See
https://github.com/go-gitea/gitea/issues/22083\#issuecomment-1347961737)
Ref #22083
Signed-off-by: Andrew Thornton <art27@cantab.net>
<!--
Please check the following:
1. Make sure you are targeting the `main` branch, pull requests on
release branches are only allowed for bug fixes.
2. Read contributing guidelines:
https://github.com/go-gitea/gitea/blob/main/CONTRIBUTING.md
3. Describe what your pull request does and which issue you're targeting
(if any)
-->
Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
- Backport of #22171
- Currently, the 'IsZero' function for 'TimeStamp' just checks if the
unix time is zero, which is not the behavior of 'Time.IsZero()', but
Gitea is using this method in accordance with the behavior of
'Time.IsZero()'.
- Adds a new condition to check for the zero time instant.
- Fixes a bug where non-expiring GPG keys where shown as they expired on
Jan 01, 0001.
- Related https://codeberg.org/Codeberg/Community/issues/791
Backport #22017
As recognised in #21841 the rendering of plain text files is somewhat
incorrect when there are ambiguous characters as the html code is double
escaped. In fact there are several more problems here.
We have a residual isRenderedHTML which is actually simply escaping the
file - not rendering it. This is badly named and gives the wrong
impression.
There is also unusual behaviour whether the file is called a Readme or
not and there is no way to get to the source code if the file is called
README.
In reality what should happen is different depending on whether the file
is being rendered a README at the bottom of the directory view or not.
1. If it is rendered as a README on a directory - it should simply be
escaped and rendered as `<pre>` text.
2. If it is rendered as a file then it should be rendered as source
code.
This PR therefore does:
1. Rename IsRenderedHTML to IsPlainText
2. Readme files rendered at the bottom of the directory are rendered
without line numbers
3. Otherwise plain text files are rendered as source code.
Replace #21841
Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Backport #22162
The PR #21198 introduced a probable security vulnerability which
resulted in making all storage files be marked as executable.
This PR ensures that these are forcibly marked as non-executable.
Fix#22161
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
Backport #22073
Gitea will attempt to lookup its location using LookPath however, this
fails on cmd.exe if gitea is in the current working directory.
exec.LookPath will return an exec.ErrDot error which we can test for and
then simply using filepath.Abs(os.Args[0]) to absolute gitea against the
current working directory.
Fix#22063
Signed-off-by: Andrew Thornton <art27@cantab.net>
Backport #21902
Although git does expect that author names should be of the form: `NAME
<EMAIL>` some users have been able to create commits with: `<EMAIL>`
Fix#21900
Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Backport #22021
When getting tracked times out of the db and loading their attributes
handle not exist errors in a nicer way. (Also prevent an NPE.)
Fix#22006
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Backport #22019
Although there are per-locale fallbacks for ambiguity the locale names
for Chinese do not quite match our locales. This PR simply maps zh-CN on
to zh-hans and other zh variants on to zh-hant.
Ref #20999
Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lauris BH <lauris@nix.lv>
Backport #21993
Paths in git are always separated by `/` not `\` - therefore we should
`path` and not `filepath`
Fix#21987
Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: Lauris BH <lauris@nix.lv>
Backport #21945
Unfortunately the fallback configuration code for [mailer] that were
added in #18982 are incorrect. When you read a value from an ini section
that key is added. This leads to a failure of the fallback mechanism.
Further there is also a spelling mistake in the startTLS configuration.
This PR restructures the mailer code to first map the deprecated
settings on to the new ones - and then use ini.MapTo to map those on to
the struct with additional validation as necessary.
Ref #21744
Signed-off-by: Andrew Thornton <art27@cantab.net>
Backport #21884
Committer avatar rendered by `func AvatarByEmail` are not vertical align
as `func Avatar` does.
- Replace literals `ui avatar` and `ui avatar vm` with the constant
`DefaultAvatarClass`
Signed-off-by: Xinyu Zhou <i@sourcehut.net>
- Backport #21856
- It's possible that the `user_redirect` table contains a user id that
no longer exists.
- Delete a user redirect upon deleting the user.
- Add a check for these dangling user redirects to check-db-consistency.
Backport #21791
There was a bug introduced in #21352 due to a change of behaviour caused
by #19280. This causes a panic on running the default doctor checks
because the panic introduced by #19280 assumes that the only way
opts.StdOut and opts.Stderr can be set in RunOpts is deliberately.
Unfortunately, when running a git.Command the provided RunOpts can be
set, therefore if you share a common set of RunOpts these two values can
be set by the previous commands.
This PR stops using common RunOpts for the commands in that doctor check
but secondly stops RunCommand variants from changing the provided
RunOpts.
Signed-off-by: Andrew Thornton <art27@cantab.net>
backport #21621
This is a performance regression from #18058
Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Andrew Thornton <art27@cantab.net>
