Skip to content

Catch os... errors #320

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 2, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions cmd/dump.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ func runDump(ctx *cli.Context) error {
log.Printf("Packing dump files...")
z, err := zip.Create(fileName)
if err != nil {
os.Remove(fileName)
log.Fatalf("Fail to create %s: %v", fileName, err)
}

Expand All @@ -102,7 +101,7 @@ func runDump(ctx *cli.Context) error {
}
// FIXME: SSH key file.
if err = z.Close(); err != nil {
os.Remove(fileName)
_ = os.Remove(fileName)
log.Fatalf("Fail to save %s: %v", fileName, err)
}

Expand All @@ -111,7 +110,10 @@ func runDump(ctx *cli.Context) error {
}

log.Printf("Removing tmp work dir: %s", TmpWorkDir)
os.RemoveAll(TmpWorkDir)

if err := os.RemoveAll(TmpWorkDir); err != nil {
log.Fatalf("Fail to remove %s: %v", TmpWorkDir, err)
}
log.Printf("Finish dumping in file %s", fileName)

return nil
Expand Down
4 changes: 3 additions & 1 deletion cmd/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ func setup(logPath string) {

if setting.UseSQLite3 || setting.UseTiDB {
workDir, _ := setting.WorkDir()
os.Chdir(workDir)
if err := os.Chdir(workDir); err != nil {
log.GitLogger.Fatal(4, "Fail to change directory %s: %v", workDir, err)
}
}

models.SetEngine()
Expand Down
5 changes: 3 additions & 2 deletions cmd/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -654,8 +654,9 @@ func runWeb(ctx *cli.Context) error {
case setting.FCGI:
err = fcgi.Serve(nil, m)
case setting.UnixSocket:
os.Remove(listenAddr)

if err := os.Remove(listenAddr); err != nil {
log.Fatal(4, "Fail to remove unix socket directory %s: %v", listenAddr, err)
}
var listener *net.UnixListener
listener, err = net.ListenUnix("unix", &net.UnixAddr{Name: listenAddr, Net: "unix"})
if err != nil {
Expand Down
8 changes: 7 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"os"
"runtime"

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

"code.gitea.io/gitea/cmd"
"code.gitea.io/gitea/modules/setting"
"github.com/urfave/cli"
Expand Down Expand Up @@ -37,5 +39,9 @@ func main() {
cmd.CmdAdmin,
}
app.Flags = append(app.Flags, []cli.Flag{}...)
app.Run(os.Args)
err := app.Run(os.Args)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally I'm wrapping this directly into println

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tboerger Don't do that. Last time, it results in a bug I spent many time to fix it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But now it's not a big difference

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not using any logger, it's only printing to stdout. So we will have the same bug again

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, don't output anything to stdout when git via ssh.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lunny how do you want me to fix this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lunny cmd/serve.go always returns nil, so this isn't really an issue here :)

if err != nil {
log.Fatal(4, "Fail to run app with %s: %v", os.Args, err)
}

}
5 changes: 4 additions & 1 deletion models/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,10 @@ func SetEngine() (err error) {
// WARNING: for serv command, MUST remove the output to os.stdout,
// so use log file to instead print to stdout.
logPath := path.Join(setting.LogRootPath, "xorm.log")
os.MkdirAll(path.Dir(logPath), os.ModePerm)

if err := os.MkdirAll(path.Dir(logPath), os.ModePerm); err != nil {
return fmt.Errorf("Fail to create dir %s: %v", logPath, err)
}

f, err := os.Create(logPath)
if err != nil {
Expand Down
11 changes: 9 additions & 2 deletions models/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,11 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository) (err error

// Clone base repo.
tmpBasePath := path.Join(setting.AppDataPath, "tmp/repos", com.ToStr(time.Now().Nanosecond())+".git")
os.MkdirAll(path.Dir(tmpBasePath), os.ModePerm)

if err := os.MkdirAll(path.Dir(tmpBasePath), os.ModePerm); err != nil {
return fmt.Errorf("Fail to create dir %s: %v", tmpBasePath, err)
}

defer os.RemoveAll(path.Dir(tmpBasePath))

var stderr string
Expand Down Expand Up @@ -622,8 +626,11 @@ func (pr *PullRequest) PushToBaseRepo() (err error) {
headFile := fmt.Sprintf("refs/pull/%d/head", pr.Index)

// Remove head in case there is a conflict.
os.Remove(path.Join(pr.BaseRepo.RepoPath(), headFile))
file := path.Join(pr.BaseRepo.RepoPath(), headFile)

if err := os.Remove(file); err != nil {
return fmt.Errorf("Fail to remove dir %s: %v", path.Join(pr.BaseRepo.RepoPath(), headFile), err)
}
if err = git.Push(headRepoPath, tmpRemoteName, fmt.Sprintf("%s:%s", pr.HeadBranch, headFile)); err != nil {
return fmt.Errorf("Push: %v", err)
}
Expand Down
30 changes: 25 additions & 5 deletions models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -558,8 +558,12 @@ func (repo *Repository) SavePatch(index int64, patch []byte) error {
if err != nil {
return fmt.Errorf("PatchPath: %v", err)
}
dir := filepath.Dir(patchPath)

if err := os.MkdirAll(dir, os.ModePerm); err != nil {
return fmt.Errorf("Fail to create dir %s: %v", dir, err)
}

os.MkdirAll(filepath.Dir(patchPath), os.ModePerm)
if err = ioutil.WriteFile(patchPath, patch, 0644); err != nil {
return fmt.Errorf("WriteFile: %v", err)
}
Expand Down Expand Up @@ -669,7 +673,10 @@ func MigrateRepository(u *User, opts MigrateRepoOptions) (*Repository, error) {

migrateTimeout := time.Duration(setting.Git.Timeout.Migrate) * time.Second

os.RemoveAll(repoPath)
if err := os.RemoveAll(repoPath); err != nil {
return repo, fmt.Errorf("Fail to remove %s: %v", repoPath, err)
}

if err = git.Clone(opts.RemoteAddr, repoPath, git.CloneRepoOptions{
Mirror: true,
Quiet: true,
Expand All @@ -680,7 +687,11 @@ func MigrateRepository(u *User, opts MigrateRepoOptions) (*Repository, error) {

wikiRemotePath := wikiRemoteURL(opts.RemoteAddr)
if len(wikiRemotePath) > 0 {
os.RemoveAll(wikiPath)

if err := os.RemoveAll(wikiPath); err != nil {
return repo, fmt.Errorf("Fail to remove %s: %v", wikiPath, err)
}

if err = git.Clone(wikiRemotePath, wikiPath, git.CloneRepoOptions{
Mirror: true,
Quiet: true,
Expand Down Expand Up @@ -902,7 +913,11 @@ func initRepository(e Engine, repoPath string, u *User, repo *Repository, opts C

// Initialize repository according to user's choice.
if opts.AutoInit {
os.MkdirAll(tmpDir, os.ModePerm)

if err := os.MkdirAll(tmpDir, os.ModePerm); err != nil {
return fmt.Errorf("Fail to create dir %s: %v", tmpDir, err)
}

defer os.RemoveAll(tmpDir)

if err = prepareRepoCommit(repo, tmpDir, repoPath, opts); err != nil {
Expand Down Expand Up @@ -1198,7 +1213,12 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error
}

// Rename remote repository to new path and delete local copy.
os.MkdirAll(UserPath(newOwner.Name), os.ModePerm)
dir := UserPath(newOwner.Name)

if err := os.MkdirAll(dir, os.ModePerm); err != nil {
return fmt.Errorf("Fail to create dir %s: %v", dir, err)
}

if err = os.Rename(RepoPath(owner.Name, repo.Name), RepoPath(newOwner.Name, repo.Name)); err != nil {
return fmt.Errorf("rename repository directory: %v", err)
}
Expand Down
18 changes: 15 additions & 3 deletions models/repo_editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,11 @@ func (repo *Repository) UpdateRepoFile(doer *User, opts UpdateRepoFileOptions) (
localPath := repo.LocalCopyPath()
oldFilePath := path.Join(localPath, opts.OldTreeName)
filePath := path.Join(localPath, opts.NewTreeName)
os.MkdirAll(path.Dir(filePath), os.ModePerm)
dir := path.Dir(filePath)

if err := os.MkdirAll(dir, os.ModePerm); err != nil {
return fmt.Errorf("Fail to create dir %s: %v", dir, err)
}

// If it's meant to be a new file, make sure it doesn't exist.
if opts.IsNewFile {
Expand Down Expand Up @@ -185,7 +189,12 @@ func (repo *Repository) GetDiffPreview(branch, treePath, content string) (diff *

localPath := repo.LocalCopyPath()
filePath := path.Join(localPath, treePath)
os.MkdirAll(filepath.Dir(filePath), os.ModePerm)
dir := filepath.Dir(filePath)

if err := os.MkdirAll(dir, os.ModePerm); err != nil {
return nil, fmt.Errorf("Fail to create dir %s: %v", dir, err)
}

if err = ioutil.WriteFile(filePath, []byte(content), 0666); err != nil {
return nil, fmt.Errorf("WriteFile: %v", err)
}
Expand Down Expand Up @@ -475,7 +484,10 @@ func (repo *Repository) UploadRepoFiles(doer *User, opts UploadRepoFileOptions)

localPath := repo.LocalCopyPath()
dirPath := path.Join(localPath, opts.TreePath)
os.MkdirAll(dirPath, os.ModePerm)

if err := os.MkdirAll(dirPath, os.ModePerm); err != nil {
return fmt.Errorf("Fail to create dir %s: %v", dirPath, err)
}

// Copy uploaded files into repository.
for _, upload := range uploads {
Expand Down
7 changes: 6 additions & 1 deletion models/ssh_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,12 @@ func addKey(e Engine, key *PublicKey) (err error) {
// Calculate fingerprint.
tmpPath := strings.Replace(path.Join(os.TempDir(), fmt.Sprintf("%d", time.Now().Nanosecond()),
"id_rsa.pub"), "\\", "/", -1)
os.MkdirAll(path.Dir(tmpPath), os.ModePerm)
dir := path.Dir(tmpPath)

if err := os.MkdirAll(dir, os.ModePerm); err != nil {
return fmt.Errorf("Fail to create dir %s: %v", dir, err)
}

if err = ioutil.WriteFile(tmpPath, []byte(key.Content), 0644); err != nil {
return err
}
Expand Down
21 changes: 17 additions & 4 deletions models/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,10 @@ func (u *User) UploadAvatar(data []byte) error {
return fmt.Errorf("updateUser: %v", err)
}

os.MkdirAll(setting.AvatarUploadPath, os.ModePerm)
if err := os.MkdirAll(setting.AvatarUploadPath, os.ModePerm); err != nil {
return fmt.Errorf("Fail to create dir %s: %v", setting.AvatarUploadPath, err)
}

fw, err := os.Create(u.CustomAvatarPath())
if err != nil {
return fmt.Errorf("Create: %v", err)
Expand All @@ -409,7 +412,10 @@ func (u *User) UploadAvatar(data []byte) error {
// DeleteAvatar deletes the user's custom avatar.
func (u *User) DeleteAvatar() error {
log.Trace("DeleteAvatar[%d]: %s", u.ID, u.CustomAvatarPath())
os.Remove(u.CustomAvatarPath())

if err := os.Remove(u.CustomAvatarPath()); err != nil {
return fmt.Errorf("Fail to remove %s: %v", u.CustomAvatarPath(), err)
}

u.UseCustomAvatar = false
if err := UpdateUser(u); err != nil {
Expand Down Expand Up @@ -866,9 +872,16 @@ func deleteUser(e *xorm.Session, u *User) error {
// FIXME: system notice
// Note: There are something just cannot be roll back,
// so just keep error logs of those operations.
path := UserPath(u.Name)

os.RemoveAll(UserPath(u.Name))
os.Remove(u.CustomAvatarPath())
if err := os.RemoveAll(path); err != nil {
return fmt.Errorf("Fail to RemoveAll %s: %v", path, err)
}

avatarPath := u.CustomAvatarPath()
if err := os.Remove(avatarPath); err != nil {
return fmt.Errorf("Fail to remove %s: %v", avatarPath, err)
}

return nil
}
Expand Down
16 changes: 13 additions & 3 deletions models/wiki.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,22 @@ func (repo *Repository) updateUncycloPage(doer *User, oldTitle, title, content, mes
return ErrUncycloAlreadyExist{filename}
}
} else {
os.Remove(path.Join(localPath, oldTitle+".md"))
file := path.Join(localPath, oldTitle+".md")

if err := os.Remove(file); err != nil {
return fmt.Errorf("Fail to remove %s: %v", file, err)
}
}

// SECURITY: if new file is a symlink to non-exist critical file,
// attack content can be written to the target file (e.g. authorized_keys2)
// as a new page operation.
// So we want to make sure the symlink is removed before write anything.
// The new file we created will be in normal text format.
os.Remove(filename)

if err := os.Remove(filename); err != nil {
return fmt.Errorf("Fail to remove %s: %v", filename, err)
}

if err = ioutil.WriteFile(filename, []byte(content), 0666); err != nil {
return fmt.Errorf("WriteFile: %v", err)
Expand Down Expand Up @@ -168,7 +175,10 @@ func (repo *Repository) DeleteUncycloPage(doer *User, title string) (err error) {

title = ToUncycloPageName(title)
filename := path.Join(localPath, title+".md")
os.Remove(filename)

if err := os.Remove(filename); err != nil {
return fmt.Errorf("Fail to remove %s: %v", filename, err)
}

message := "Delete page '" + title + "'"

Expand Down
5 changes: 4 additions & 1 deletion modules/log/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,10 @@ func (w *FileLogWriter) deleteOldLog() {

if !info.IsDir() && info.ModTime().Unix() < (time.Now().Unix()-60*60*24*w.Maxdays) {
if strings.HasPrefix(filepath.Base(path), filepath.Base(w.Filename)) {
os.Remove(path)

if err := os.Remove(path); err != nil {
returnErr = fmt.Errorf("Fail to remove %s: %v", path, err)
}
}
}
return returnErr
Expand Down
7 changes: 6 additions & 1 deletion modules/log/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,12 @@ func NewLogger(bufLen int64, mode, config string) {
// NewGitLogger create a logger for git
// FIXME: use same log level as other loggers.
func NewGitLogger(logPath string) {
os.MkdirAll(path.Dir(logPath), os.ModePerm)
path := path.Dir(logPath)

if err := os.MkdirAll(path, os.ModePerm); err != nil {
Fatal(4, "Fail to create dir %s: %v", path, err)
}

GitLogger = newLogger(0)
GitLogger.SetLogger("file", fmt.Sprintf(`{"level":0,"filename":"%s","rotate":false}`, logPath))
}
Expand Down
7 changes: 6 additions & 1 deletion modules/ssh/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,12 @@ func Listen(port int) {

keyPath := filepath.Join(setting.AppDataPath, "ssh/gogs.rsa")
if !com.IsExist(keyPath) {
os.MkdirAll(filepath.Dir(keyPath), os.ModePerm)
filePath := filepath.Dir(keyPath)

if err := os.MkdirAll(filePath, os.ModePerm); err != nil {
log.Error(4, "Fail to create dir %s: %v", filePath, err)
}

_, stderr, err := com.ExecCmd("ssh-keygen", "-f", keyPath, "-t", "rsa", "-N", "")
if err != nil {
panic(fmt.Sprintf("Fail to generate private key: %v - %s", err, stderr))
Expand Down