Skip to content

Commit

Permalink
Revert "Use CleanPath instead of path.Clean (#23371)"
Browse files Browse the repository at this point in the history
This reverts commit b116418.
  • Loading branch information
lunny authored Mar 15, 2023
1 parent 0d7cf7b commit d5c2743
Show file tree
Hide file tree
Showing 13 changed files with 28 additions and 45 deletions.
12 changes: 8 additions & 4 deletions models/git/lfs_lock.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package git
import (
"context"
"fmt"
"path"
"strings"
"time"

Expand All @@ -16,7 +17,6 @@ import (
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
)

// LFSLock represents a git lfs lock of repository.
Expand All @@ -34,7 +34,11 @@ func init() {

// BeforeInsert is invoked from XORM before inserting an object of this type.
func (l *LFSLock) BeforeInsert() {
l.Path = util.CleanPath(l.Path)
l.Path = cleanPath(l.Path)
}

func cleanPath(p string) string {
return path.Clean("/" + p)[1:]
}

// CreateLFSLock creates a new lock.
Expand All @@ -49,7 +53,7 @@ func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLo
return nil, err
}

lock.Path = util.CleanPath(lock.Path)
lock.Path = cleanPath(lock.Path)
lock.RepoID = repo.ID

l, err := GetLFSLock(dbCtx, repo, lock.Path)
Expand All @@ -69,7 +73,7 @@ func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLo

// GetLFSLock returns release by given path.
func GetLFSLock(ctx context.Context, repo *repo_model.Repository, path string) (*LFSLock, error) {
path = util.CleanPath(path)
path = cleanPath(path)
rel := &LFSLock{RepoID: repo.ID}
has, err := db.GetEngine(ctx).Where("lower(path) = ?", strings.ToLower(path)).Get(rel)
if err != nil {
Expand Down
10 changes: 5 additions & 5 deletions modules/options/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,27 +16,27 @@ import (

// Locale reads the content of a specific locale from static/bindata or custom path.
func Locale(name string) ([]byte, error) {
return fileFromDir(path.Join("locale", util.CleanPath(name)))
return fileFromDir(path.Join("locale", path.Clean("/"+name)))
}

// Readme reads the content of a specific readme from static/bindata or custom path.
func Readme(name string) ([]byte, error) {
return fileFromDir(path.Join("readme", util.CleanPath(name)))
return fileFromDir(path.Join("readme", path.Clean("/"+name)))
}

// Gitignore reads the content of a gitignore locale from static/bindata or custom path.
func Gitignore(name string) ([]byte, error) {
return fileFromDir(path.Join("gitignore", util.CleanPath(name)))
return fileFromDir(path.Join("gitignore", path.Clean("/"+name)))
}

// License reads the content of a specific license from static/bindata or custom path.
func License(name string) ([]byte, error) {
return fileFromDir(path.Join("license", util.CleanPath(name)))
return fileFromDir(path.Join("license", path.Clean("/"+name)))
}

// Labels reads the content of a specific labels from static/bindata or custom path.
func Labels(name string) ([]byte, error) {
return fileFromDir(path.Join("label", util.CleanPath(name)))
return fileFromDir(path.Join("label", path.Clean("/"+name)))
}

// WalkLocales reads the content of a specific locale
Expand Down
4 changes: 2 additions & 2 deletions modules/public/public.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ package public
import (
"net/http"
"os"
"path"
"path/filepath"
"strings"

"code.gitea.io/gitea/modules/container"
"code.gitea.io/gitea/modules/httpcache"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
)

// Options represents the available options to configure the handler.
Expand Down Expand Up @@ -103,7 +103,7 @@ func setWellKnownContentType(w http.ResponseWriter, file string) {

func (opts *Options) handle(w http.ResponseWriter, req *http.Request, fs http.FileSystem, file string) bool {
// use clean to keep the file is a valid path with no . or ..
f, err := fs.Open(util.CleanPath(file))
f, err := fs.Open(path.Clean(file))
if err != nil {
if os.IsNotExist(err) {
return false
Expand Down
3 changes: 2 additions & 1 deletion modules/storage/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"io"
"net/url"
"os"
"path"
"path/filepath"
"strings"

Expand Down Expand Up @@ -58,7 +59,7 @@ func NewLocalStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error
}

func (l *LocalStorage) buildLocalPath(p string) string {
return filepath.Join(l.dir, util.CleanPath(strings.ReplaceAll(p, "\\", "/")))
return filepath.Join(l.dir, path.Clean("/" + strings.ReplaceAll(p, "\\", "/"))[1:])
}

// Open a file
Expand Down
3 changes: 1 addition & 2 deletions modules/storage/minio.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"time"

"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/util"

"github.com/minio/minio-go/v7"
"github.com/minio/minio-go/v7/pkg/credentials"
Expand Down Expand Up @@ -121,7 +120,7 @@ func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error
}

func (m *MinioStorage) buildMinioPath(p string) string {
return strings.TrimPrefix(path.Join(m.basePath, util.CleanPath(strings.ReplaceAll(p, "\\", "/"))), "/")
return strings.TrimPrefix(path.Join(m.basePath, path.Clean("/" + strings.ReplaceAll(p, "\\", "/"))[1:]), "/")
}

// Open open a file
Expand Down
8 changes: 0 additions & 8 deletions modules/util/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,6 @@ import (
"strings"
)

// CleanPath ensure to clean the path
func CleanPath(p string) string {
if strings.HasPrefix(p, "/") {
return path.Clean(p)
}
return path.Clean("/" + p)[1:]
}

// EnsureAbsolutePath ensure that a path is absolute, making it
// relative to absoluteBase if necessary
func EnsureAbsolutePath(path, absoluteBase string) string {
Expand Down
12 changes: 0 additions & 12 deletions modules/util/path_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,15 +136,3 @@ func TestMisc_IsReadmeFileName(t *testing.T) {
assert.Equal(t, testCase.idx, idx)
}
}

func TestCleanPath(t *testing.T) {
cases := map[string]string{
"../../test": "test",
"/test": "/test",
"/../test": "/test",
}

for k, v := range cases {
assert.Equal(t, v, CleanPath(k))
}
}
5 changes: 2 additions & 3 deletions routers/web/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/storage"
"code.gitea.io/gitea/modules/templates"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/modules/web/middleware"
"code.gitea.io/gitea/modules/web/routing"
"code.gitea.io/gitea/services/auth"
Expand All @@ -45,7 +44,7 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor
routing.UpdateFuncInfo(req.Context(), funcInfo)

rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/")
rPath = util.CleanPath(strings.ReplaceAll(rPath, "\\", "/"))
rPath = path.Clean("/" + strings.ReplaceAll(rPath, "\\", "/"))[1:]

u, err := objStore.URL(rPath, path.Base(rPath))
if err != nil {
Expand Down Expand Up @@ -81,7 +80,7 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor
routing.UpdateFuncInfo(req.Context(), funcInfo)

rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/")
rPath = util.CleanPath(strings.ReplaceAll(rPath, "\\", "/"))
rPath = path.Clean("/" + strings.ReplaceAll(rPath, "\\", "/"))[1:]
if rPath == "" {
http.Error(w, "file not found", http.StatusNotFound)
return
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ func UploadFilePost(ctx *context.Context) {

func cleanUploadFileName(name string) string {
// Rebase the filename
name = strings.Trim(util.CleanPath(name), "/")
name = strings.Trim(path.Clean("/"+name), "/")
// Git disallows any filenames to have a .git directory in them.
for _, part := range strings.Split(name, "/") {
if strings.ToLower(part) == ".git" {
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/lfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func LFSLockFile(ctx *context.Context) {
ctx.Redirect(ctx.Repo.RepoLink + "/settings/lfs/locks")
return
}
lockPath = util.CleanPath(lockPath)
lockPath = path.Clean("/" + lockPath)[1:]
if len(lockPath) == 0 {
ctx.Flash.Error(ctx.Tr("repo.settings.lfs_invalid_locking_path", originalPath))
ctx.Redirect(ctx.Repo.RepoLink + "/settings/lfs/locks")
Expand Down
4 changes: 2 additions & 2 deletions services/migrations/gitea_uploader.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"io"
"os"
"path"
"path/filepath"
"strconv"
"strings"
Expand All @@ -29,7 +30,6 @@ import (
"code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/uri"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/services/pull"

"github.com/google/uuid"
Expand Down Expand Up @@ -866,7 +866,7 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error {
}

// SECURITY: The TreePath must be cleaned!
comment.TreePath = util.CleanPath(comment.TreePath)
comment.TreePath = path.Clean("/" + comment.TreePath)[1:]

var patch string
reader, writer := io.Pipe()
Expand Down
4 changes: 2 additions & 2 deletions services/packages/container/blob_uploader.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ import (
"errors"
"io"
"os"
"path"
"path/filepath"
"strings"

packages_model "code.gitea.io/gitea/models/packages"
packages_module "code.gitea.io/gitea/modules/packages"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
)

var (
Expand All @@ -33,7 +33,7 @@ type BlobUploader struct {
}

func buildFilePath(id string) string {
return filepath.Join(setting.Packages.ChunkedUploadPath, util.CleanPath(strings.ReplaceAll(id, "\\", "/")))
return filepath.Join(setting.Packages.ChunkedUploadPath, path.Clean("/" + strings.ReplaceAll(id, "\\", "/"))[1:])
}

// NewBlobUploader creates a new blob uploader for the given id
Expand Down
4 changes: 2 additions & 2 deletions services/repository/files/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ import (
"context"
"fmt"
"net/url"
"path"
"strings"
"time"

repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util"
)

// GetFileResponseFromCommit Constructs a FileResponse from a Commit object
Expand Down Expand Up @@ -129,7 +129,7 @@ func GetAuthorAndCommitterUsers(author, committer *IdentityOptions, doer *user_m
// CleanUploadFileName Trims a filename and returns empty string if it is a .git directory
func CleanUploadFileName(name string) string {
// Rebase the filename
name = strings.Trim(util.CleanPath(name), "/")
name = strings.Trim(path.Clean("/"+name), "/")
// Git disallows any filenames to have a .git directory in them.
for _, part := range strings.Split(name, "/") {
if strings.ToLower(part) == ".git" {
Expand Down

0 comments on commit d5c2743

Please sign in to comment.