Merge pull request #1190 from tw4452852/1188

errors: don't join the absolute file path

caddyhttp/errors/setup.go

@@ -115,7 +115,9 @@ func errorsParse(c *caddy.Controller) (*ErrorHandler, error) {
 				}
 			} else {
 				// Error page; ensure it exists
-				where = filepath.Join(cfg.Root, where)
+				if !filepath.IsAbs(where) {
+					where = filepath.Join(cfg.Root, where)
+				}
 				f, err := os.Open(where)
 				if err != nil {
 					log.Printf("[WARNING] Unable to open error page '%s': %v", where, err)

caddyhttp/errors/setup_test.go

 package errors
 
 import (
+	"path/filepath"
+	"reflect"
 	"testing"
 
 	"github.com/mholt/caddy"
@@ -45,24 +47,29 @@ func TestSetup(t *testing.T) {
 }
 
 func TestErrorsParse(t *testing.T) {
+	testAbs, err := filepath.Abs("./404.html")
+	if err != nil {
+		t.Error(err)
+	}
 	tests := []struct {
 		inputErrorsRules     string
 		shouldErr            bool
 		expectedErrorHandler ErrorHandler
 	}{
 		{`errors`, false, ErrorHandler{
-			LogFile: "",
+			ErrorPages: map[int]string{},
 		}},
 		{`errors errors.txt`, false, ErrorHandler{
-			LogFile: "errors.txt",
+			ErrorPages: map[int]string{},
+			LogFile:    "errors.txt",
 		}},
 		{`errors visible`, false, ErrorHandler{
-			LogFile: "",
-			Debug:   true,
+			ErrorPages: map[int]string{},
+			Debug:      true,
 		}},
 		{`errors { log visible }`, false, ErrorHandler{
-			LogFile: "",
-			Debug:   true,
+			ErrorPages: map[int]string{},
+			Debug:      true,
 		}},
 		{`errors { log errors.txt
         404 404.html
@@ -82,6 +89,7 @@ func TestErrorsParse(t *testing.T) {
 				MaxBackups: 3,
 				LocalTime:  true,
 			},
+			ErrorPages: map[int]string{},
 		}},
 		{`errors { log errors.txt {
             size 3
@@ -115,15 +123,24 @@ func TestErrorsParse(t *testing.T) {
 				503: "503.html",
 			},
 		}},
+		// test absolute file path
+		{`errors {
+			404 ` + testAbs + `
+		}`,
+			false, ErrorHandler{
+				ErrorPages: map[int]string{
+					404: testAbs,
+				},
+			}},
 		// Next two test cases is the detection of duplicate status codes
 		{`errors {
         503 503.html
         503 503.html
-}`, true, ErrorHandler{}},
+}`, true, ErrorHandler{ErrorPages: map[int]string{}}},
 		{`errors {
         * generic_error.html
         * generic_error.html
-}`, true, ErrorHandler{}},
+}`, true, ErrorHandler{ErrorPages: map[int]string{}}},
 	}
 	for i, test := range tests {
 		actualErrorsRule, err := errorsParse(caddy.NewTestController("http", test.inputErrorsRules))
@@ -135,48 +152,9 @@ func TestErrorsParse(t *testing.T) {
 		} else if err != nil && test.shouldErr {
 			continue
 		}
-		if actualErrorsRule.LogFile != test.expectedErrorHandler.LogFile {
-			t.Errorf("Test %d expected LogFile to be %s, but got %s",
-				i, test.expectedErrorHandler.LogFile, actualErrorsRule.LogFile)
-		}
-		if actualErrorsRule.Debug != test.expectedErrorHandler.Debug {
-			t.Errorf("Test %d expected Debug to be %v, but got %v",
-				i, test.expectedErrorHandler.Debug, actualErrorsRule.Debug)
-		}
-		if actualErrorsRule.LogRoller != nil && test.expectedErrorHandler.LogRoller == nil || actualErrorsRule.LogRoller == nil && test.expectedErrorHandler.LogRoller != nil {
-			t.Fatalf("Test %d expected LogRoller to be %v, but got %v",
-				i, test.expectedErrorHandler.LogRoller, actualErrorsRule.LogRoller)
-		}
-		if len(actualErrorsRule.ErrorPages) != len(test.expectedErrorHandler.ErrorPages) {
-			t.Fatalf("Test %d expected %d no of Error pages, but got %d ",
-				i, len(test.expectedErrorHandler.ErrorPages), len(actualErrorsRule.ErrorPages))
-		}
-		if actualErrorsRule.LogRoller != nil && test.expectedErrorHandler.LogRoller != nil {
-			if actualErrorsRule.LogRoller.Filename != test.expectedErrorHandler.LogRoller.Filename {
-				t.Fatalf("Test %d expected LogRoller Filename to be %s, but got %s",
-					i, test.expectedErrorHandler.LogRoller.Filename, actualErrorsRule.LogRoller.Filename)
-			}
-			if actualErrorsRule.LogRoller.MaxAge != test.expectedErrorHandler.LogRoller.MaxAge {
-				t.Fatalf("Test %d expected LogRoller MaxAge to be %d, but got %d",
-					i, test.expectedErrorHandler.LogRoller.MaxAge, actualErrorsRule.LogRoller.MaxAge)
-			}
-			if actualErrorsRule.LogRoller.MaxBackups != test.expectedErrorHandler.LogRoller.MaxBackups {
-				t.Fatalf("Test %d expected LogRoller MaxBackups to be %d, but got %d",
-					i, test.expectedErrorHandler.LogRoller.MaxBackups, actualErrorsRule.LogRoller.MaxBackups)
-			}
-			if actualErrorsRule.LogRoller.MaxSize != test.expectedErrorHandler.LogRoller.MaxSize {
-				t.Fatalf("Test %d expected LogRoller MaxSize to be %d, but got %d",
-					i, test.expectedErrorHandler.LogRoller.MaxSize, actualErrorsRule.LogRoller.MaxSize)
-			}
-			if actualErrorsRule.LogRoller.LocalTime != test.expectedErrorHandler.LogRoller.LocalTime {
-				t.Fatalf("Test %d expected LogRoller LocalTime to be %t, but got %t",
-					i, test.expectedErrorHandler.LogRoller.LocalTime, actualErrorsRule.LogRoller.LocalTime)
-			}
-		}
-		if actualErrorsRule.GenericErrorPage != test.expectedErrorHandler.GenericErrorPage {
-			t.Fatalf("Test %d expected GenericErrorPage to be %v, but got %v",
-				i, test.expectedErrorHandler.GenericErrorPage, actualErrorsRule.GenericErrorPage)
+		if !reflect.DeepEqual(actualErrorsRule, &test.expectedErrorHandler) {
+			t.Errorf("Test %d expect %v, but got %v", i,
+				actualErrorsRule, test.expectedErrorHandler)
 		}
 	}
-
 }