From 2bd6fd0aea6a203aecc5ea6e1f3257b990697d0c Mon Sep 17 00:00:00 2001 From: Tw Date: Tue, 18 Oct 2016 09:16:26 +0800 Subject: [PATCH] errors: don't join the absolute file path fix issue #1188 Signed-off-by: Tw --- caddyhttp/errors/setup.go | 4 +- caddyhttp/errors/setup_test.go | 78 ++++++++++++---------------------- 2 files changed, 31 insertions(+), 51 deletions(-) diff --git a/caddyhttp/errors/setup.go b/caddyhttp/errors/setup.go index ecaa92464..3d643e52b 100644 --- a/caddyhttp/errors/setup.go +++ b/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) diff --git a/caddyhttp/errors/setup_test.go b/caddyhttp/errors/setup_test.go index 24942cd89..9aa861d55 100644 --- a/caddyhttp/errors/setup_test.go +++ b/caddyhttp/errors/setup_test.go @@ -1,6 +1,8 @@ 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) } } - }