[tor-commits] [goptlib/master] Implement MakeStateDir() instead of messing with the info structs.

[yawning at torproject.org]

commit 6d42c7b0e3902ad8b40b17ef096bb79d50ec56f1
Author: Yawning Angel <yawning at schwanenlied.me>
Date:   Fri May 23 06:43:24 2014 +0000

    Implement MakeStateDir() instead of messing with the info structs.
    
    This fixes bug #12088.
---
 pt.go      |   35 ++++++++++-------------------------
 pt_test.go |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 25 deletions(-)

diff --git a/pt.go b/pt.go
index 7f6c0a7..dba6701 100644
--- a/pt.go
+++ b/pt.go
@@ -322,20 +322,17 @@ func getManagedTransportVer() (string, error) {
 	return "", versionError("no-version")
 }
 
-// Get the pluggable transport state location offered by Tor, and create it if
-// missing.  This function reads the enviornment variable
-// TOR_PT_STATE_LOCATION.
-func getStateLocation() (string, error) {
-	stateLocation, err := getenvRequired("TOR_PT_STATE_LOCATION")
+// Return the directory name in the TOR_PT_STATE_LOCATION environment variable,
+// creating it if it doesn't exist. Returns non-nil error if
+// TOR_PT_STATE_LOCATION is not set or if there is an error creating the
+// directory.
+func MakeStateDir() (string, error) {
+	dir, err := getenvRequired("TOR_PT_STATE_LOCATION")
 	if err != nil {
 		return "", err
 	}
-	err = os.MkdirAll(stateLocation, 0700)
-	if err != nil {
-		return "", envError(fmt.Sprintf("error creating TOR_PT_STATE_LOCATION: %s", err))
-	}
-
-	return stateLocation, nil
+	err = os.MkdirAll(dir, 0700)
+	return dir, err
 }
 
 // Get the intersection of the method names offered by Tor and those in
@@ -353,10 +350,9 @@ func getClientTransports(star []string) ([]string, error) {
 }
 
 // This structure is returned by ClientSetup. It consists of a list of method
-// names and the state location.
+// names.
 type ClientInfo struct {
 	MethodNames   []string
-	StateLocation string
 }
 
 // Check the client pluggable transports environment, emitting an error message
@@ -370,11 +366,6 @@ func ClientSetup(star []string) (info ClientInfo, err error) {
 	}
 	line("VERSION", ver)
 
-	info.StateLocation, err = getStateLocation()
-	if err != nil {
-		return
-	}
-
 	info.MethodNames, err = getClientTransports(star)
 	if err != nil {
 		return
@@ -540,13 +531,12 @@ func readAuthCookieFile(filename string) ([]byte, error) {
 
 // This structure is returned by ServerSetup. It consists of a list of
 // Bindaddrs, an address for the ORPort, an address for the extended ORPort (if
-// any), an authentication cookie (if any), and the state location.
+// any), and an authentication cookie (if any).
 type ServerInfo struct {
 	Bindaddrs      []Bindaddr
 	OrAddr         *net.TCPAddr
 	ExtendedOrAddr *net.TCPAddr
 	AuthCookie     []byte
-	StateLocation  string
 }
 
 // Check the server pluggable transports environment, emitting an error message
@@ -561,11 +551,6 @@ func ServerSetup(star []string) (info ServerInfo, err error) {
 	}
 	line("VERSION", ver)
 
-	info.StateLocation, err = getStateLocation()
-	if err != nil {
-		return
-	}
-
 	info.Bindaddrs, err = getServerBindaddrs(star)
 	if err != nil {
 		return
diff --git a/pt_test.go b/pt_test.go
index 5689405..b569352 100644
--- a/pt_test.go
+++ b/pt_test.go
@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"net"
 	"os"
+	"path"
 	"sort"
 	"testing"
 )
@@ -738,6 +739,64 @@ func TestExtOrPortSetup(t *testing.T) {
 	testExtOrPortSetupIndividual(t, addr, methodName)
 }
 
+func TestMakeStateDir(t *testing.T) {
+	os.Clearenv()
+
+	// TOR_PT_STATE_LOCATION not set.
+	_, err := MakeStateDir()
+	if err == nil {
+		t.Errorf("empty environment unexpectedly succeeded")
+	}
+
+	// Setup the scratch directory.
+	tempDir, err := ioutil.TempDir("", "testMakeStateDir")
+	if err != nil {
+		t.Fatalf("ioutil.TempDir failed: %s", err)
+	}
+	defer os.RemoveAll(tempDir)
+
+	goodTests := [...]string {
+		// Already existing directory.
+		tempDir,
+
+		// Nonexistent directory, parent exists.
+		path.Join(tempDir, "parentExists"),
+
+		// Nonexistent directory, parent doesn't exist.
+		path.Join(tempDir, "missingParent", "parentMissing"),
+	}
+	for _, test := range goodTests {
+		os.Setenv("TOR_PT_STATE_LOCATION", test)
+		dir, err := MakeStateDir()
+		if err != nil {
+			t.Errorf("MakeStateDir unexpectedly failed: %s", err)
+		}
+		if dir != test {
+			t.Errorf("MakeStateDir returned an unexpected path %s (expecting %s)", dir, test)
+		}
+	}
+
+	// Name already exists, but is an ordinary file.
+	tempFile := path.Join(tempDir, "file")
+	f, err := os.Create(tempFile)
+	if err != nil {
+		t.Fatalf("os.Create failed: %s", err)
+	}
+	defer f.Close()
+	os.Setenv("TOR_PT_STATE_LOCATION", tempFile)
+	_, err = MakeStateDir()
+	if err == nil {
+		t.Errorf("MakeStateDir with a file unexpectedly succeded")
+	}
+
+	// Directory name that cannot be created. (Subdir of a file)
+	os.Setenv("TOR_PT_STATE_LOCATION", path.Join(tempFile, "subDir"))
+	_, err = MakeStateDir()
+	if err == nil {
+		t.Errorf("MakeStateDir with a subdirectory of a file unexpectedly succeded")
+	}
+}
+
 func TestIsClient(t *testing.T) {
 	os.Clearenv()
 	if IsClient() {