Browse Source

Try not to leave directories behind with incorrect permissions

Jakob Borg 11 years ago
parent
commit
fe43e3b89d
3 changed files with 29 additions and 7 deletions
  1. 7 4
      integration/all.sh
  2. 9 1
      model/puller.go
  3. 13 2
      osutil/osutil.go

+ 7 - 4
integration/all.sh

@@ -1,5 +1,8 @@
-#!/bin/sh
+#!/bin/bash
+set -euo pipefail
+IFS=$'\n\t'
 
-./test-http.sh || exit
-./test-merge.sh || exit
-./test-delupd.sh || exit
+go test -tags integration -v
+./test-http.sh
+./test-merge.sh
+./test-delupd.sh

+ 9 - 1
model/puller.go

@@ -461,7 +461,7 @@ func (p *puller) handleBlock(b bqBlock) bool {
 		of.temp = filepath.Join(p.repoCfg.Directory, defTempNamer.TempName(f.Name))
 
 		dirName := filepath.Dir(of.filepath)
-		_, err := os.Stat(dirName)
+		info, err := os.Stat(dirName)
 		if err != nil {
 			err = os.MkdirAll(dirName, 0777)
 		} else {
@@ -469,6 +469,8 @@ func (p *puller) handleBlock(b bqBlock) bool {
 			if dirName != p.repoCfg.Directory {
 				err = os.Chmod(dirName, 0777)
 			}
+			// Change it back after creating the file, to minimize the time window with incorrect permissions
+			defer os.Chmod(dirName, info.Mode())
 		}
 		if err != nil {
 			l.Infof("mkdir: error: %q / %q: %v", p.repoCfg.ID, f.Name, err)
@@ -632,7 +634,13 @@ func (p *puller) handleEmptyBlock(b bqBlock) {
 		dirName := filepath.Dir(of.filepath)
 		os.Chmod(of.filepath, 0666)
 		if dirName != p.repoCfg.Directory {
+			info, err := os.Stat(dirName)
+			if err != nil {
+				l.Debugln("weird! can't happen?", err)
+			}
 			os.Chmod(dirName, 0777)
+			// Change it back after deleting the file, to minimize the time window with incorrect permissions
+			defer os.Chmod(dirName, info.Mode())
 		}
 		if p.versioner != nil {
 			if debug {

+ 13 - 2
osutil/osutil.go

@@ -7,17 +7,28 @@ package osutil
 
 import (
 	"os"
+	"path/filepath"
 	"runtime"
 )
 
 func Rename(from, to string) error {
+	// Make sure the destination directory is writeable
+	toDir := filepath.Dir(to)
+	if info, err := os.Stat(toDir); err == nil {
+		os.Chmod(toDir, 0777)
+		defer os.Chmod(toDir, info.Mode())
+	}
+
+	// On Windows, make sure the destination file is writeable (or we can't delete it)
 	if runtime.GOOS == "windows" {
-		os.Chmod(to, 0666) // Make sure the file is user writeable
+		os.Chmod(to, 0666)
 		err := os.Remove(to)
 		if err != nil && !os.IsNotExist(err) {
 			return err
 		}
 	}
-	defer os.Remove(from) // Don't leave a dangling temp file in case of rename error
+
+	// Don't leave a dangling temp file in case of rename error
+	defer os.Remove(from)
 	return os.Rename(from, to)
 }