mirror of
				https://github.com/caddyserver/caddy.git
				synced 2025-10-31 10:37:24 -04:00 
			
		
		
		
	redir: loading block arguments before parsing matcher
fix issue #977 Signed-off-by: Tw <tw19881113@gmail.com>
This commit is contained in:
		
							parent
							
								
									7157bdc79d
								
							
						
					
					
						commit
						b64894c31e
					
				| @ -33,15 +33,49 @@ func redirParse(c *caddy.Controller) ([]Rule, error) { | ||||
| 
 | ||||
| 	cfg := httpserver.GetConfig(c) | ||||
| 
 | ||||
| 	// setRedirCode sets the redirect code for rule if it can, or returns an error | ||||
| 	setRedirCode := func(code string, rule *Rule) error { | ||||
| 	initRule := func(rule *Rule, defaultCode string, args []string) error { | ||||
| 		if cfg.TLS.Enabled { | ||||
| 			rule.FromScheme = "https" | ||||
| 		} else { | ||||
| 			rule.FromScheme = "http" | ||||
| 		} | ||||
| 
 | ||||
| 		var ( | ||||
| 			from = "/" | ||||
| 			to   string | ||||
| 			code = defaultCode | ||||
| 		) | ||||
| 		switch len(args) { | ||||
| 		case 1: | ||||
| 			// To specified (catch-all redirect) | ||||
| 			// Not sure why user is doing this in a table, as it causes all other redirects to be ignored. | ||||
| 			// As such, this feature remains undocumented. | ||||
| 			to = args[0] | ||||
| 		case 2: | ||||
| 			// From and To specified | ||||
| 			from = args[0] | ||||
| 			to = args[1] | ||||
| 		case 3: | ||||
| 			// From, To, and Code specified | ||||
| 			from = args[0] | ||||
| 			to = args[1] | ||||
| 			code = args[2] | ||||
| 		default: | ||||
| 			return c.ArgErr() | ||||
| 		} | ||||
| 
 | ||||
| 		rule.FromPath = from | ||||
| 		rule.To = to | ||||
| 		if code == "meta" { | ||||
| 			rule.Meta = true | ||||
| 		} else if codeNumber, ok := httpRedirs[code]; ok { | ||||
| 			code = defaultCode | ||||
| 		} | ||||
| 		if codeNumber, ok := httpRedirs[code]; ok { | ||||
| 			rule.Code = codeNumber | ||||
| 		} else { | ||||
| 			return c.Errf("Invalid redirect code '%v'", code) | ||||
| 		} | ||||
| 
 | ||||
| 		return nil | ||||
| 	} | ||||
| 
 | ||||
| @ -62,12 +96,14 @@ func redirParse(c *caddy.Controller) ([]Rule, error) { | ||||
| 		return nil | ||||
| 	} | ||||
| 
 | ||||
| 	const initDefaultCode = "301" | ||||
| 
 | ||||
| 	for c.Next() { | ||||
| 		args := c.RemainingArgs() | ||||
| 		matcher, err := httpserver.SetupIfMatcher(c) | ||||
| 		if err != nil { | ||||
| 			return nil, err | ||||
| 		} | ||||
| 		args := c.RemainingArgs() | ||||
| 
 | ||||
| 		var hadOptionalBlock bool | ||||
| 		for c.NextBlock() { | ||||
| @ -77,103 +113,40 @@ func redirParse(c *caddy.Controller) ([]Rule, error) { | ||||
| 
 | ||||
| 			hadOptionalBlock = true | ||||
| 
 | ||||
| 			var rule = Rule{ | ||||
| 			rule := Rule{ | ||||
| 				RequestMatcher: matcher, | ||||
| 			} | ||||
| 
 | ||||
| 			if cfg.TLS.Enabled { | ||||
| 				rule.FromScheme = "https" | ||||
| 			} else { | ||||
| 				rule.FromScheme = "http" | ||||
| 			} | ||||
| 
 | ||||
| 			defaultCode := initDefaultCode | ||||
| 			// Set initial redirect code | ||||
| 			// BUG: If the code is specified for a whole block and that code is invalid, | ||||
| 			// the line number will appear on the first line inside the block, even if that | ||||
| 			// line overwrites the block-level code with a valid redirect code. The program | ||||
| 			// still functions correctly, but the line number in the error reporting is | ||||
| 			// misleading to the user. | ||||
| 			if len(args) == 1 { | ||||
| 				err := setRedirCode(args[0], &rule) | ||||
| 				if err != nil { | ||||
| 					return redirects, err | ||||
| 				} | ||||
| 			} else { | ||||
| 				rule.Code = http.StatusMovedPermanently // default code | ||||
| 				defaultCode = args[0] | ||||
| 			} | ||||
| 
 | ||||
| 			// RemainingArgs only gets the values after the current token, but in our | ||||
| 			// case we want to include the current token to get an accurate count. | ||||
| 			insideArgs := append([]string{c.Val()}, c.RemainingArgs()...) | ||||
| 
 | ||||
| 			switch len(insideArgs) { | ||||
| 			case 1: | ||||
| 				// To specified (catch-all redirect) | ||||
| 				// Not sure why user is doing this in a table, as it causes all other redirects to be ignored. | ||||
| 				// As such, this feature remains undocumented. | ||||
| 				rule.FromPath = "/" | ||||
| 				rule.To = insideArgs[0] | ||||
| 			case 2: | ||||
| 				// From and To specified | ||||
| 				rule.FromPath = insideArgs[0] | ||||
| 				rule.To = insideArgs[1] | ||||
| 			case 3: | ||||
| 				// From, To, and Code specified | ||||
| 				rule.FromPath = insideArgs[0] | ||||
| 				rule.To = insideArgs[1] | ||||
| 				err := setRedirCode(insideArgs[2], &rule) | ||||
| 				if err != nil { | ||||
| 					return redirects, err | ||||
| 				} | ||||
| 			default: | ||||
| 				return redirects, c.ArgErr() | ||||
| 			err := initRule(&rule, defaultCode, insideArgs) | ||||
| 			if err != nil { | ||||
| 				return redirects, err | ||||
| 			} | ||||
| 
 | ||||
| 			err := checkAndSaveRule(rule) | ||||
| 			err = checkAndSaveRule(rule) | ||||
| 			if err != nil { | ||||
| 				return redirects, err | ||||
| 			} | ||||
| 		} | ||||
| 
 | ||||
| 		if !hadOptionalBlock { | ||||
| 			var rule = Rule{ | ||||
| 			rule := Rule{ | ||||
| 				RequestMatcher: matcher, | ||||
| 			} | ||||
| 
 | ||||
| 			if cfg.TLS.Enabled { | ||||
| 				rule.FromScheme = "https" | ||||
| 			} else { | ||||
| 				rule.FromScheme = "http" | ||||
| 			err := initRule(&rule, initDefaultCode, args) | ||||
| 			if err != nil { | ||||
| 				return redirects, err | ||||
| 			} | ||||
| 
 | ||||
| 			rule.Code = http.StatusMovedPermanently // default | ||||
| 
 | ||||
| 			switch len(args) { | ||||
| 			case 1: | ||||
| 				// To specified (catch-all redirect) | ||||
| 				rule.FromPath = "/" | ||||
| 				rule.To = args[0] | ||||
| 			case 2: | ||||
| 				// To and Code specified (catch-all redirect) | ||||
| 				rule.FromPath = "/" | ||||
| 				rule.To = args[0] | ||||
| 				err := setRedirCode(args[1], &rule) | ||||
| 				if err != nil { | ||||
| 					return redirects, err | ||||
| 				} | ||||
| 			case 3: | ||||
| 				// From, To, and Code specified | ||||
| 				rule.FromPath = args[0] | ||||
| 				rule.To = args[1] | ||||
| 				err := setRedirCode(args[2], &rule) | ||||
| 				if err != nil { | ||||
| 					return redirects, err | ||||
| 				} | ||||
| 			default: | ||||
| 				return redirects, c.ArgErr() | ||||
| 			} | ||||
| 
 | ||||
| 			err := checkAndSaveRule(rule) | ||||
| 			err = checkAndSaveRule(rule) | ||||
| 			if err != nil { | ||||
| 				return redirects, err | ||||
| 			} | ||||
|  | ||||
| @ -1,6 +1,7 @@ | ||||
| package redirect | ||||
| 
 | ||||
| import ( | ||||
| 	"fmt" | ||||
| 	"testing" | ||||
| 
 | ||||
| 	"github.com/mholt/caddy" | ||||
| @ -15,7 +16,7 @@ func TestSetup(t *testing.T) { | ||||
| 		expectedRules []Rule | ||||
| 	}{ | ||||
| 		// test case #0 tests the recognition of a valid HTTP status code defined outside of block statement | ||||
| 		{"redir 300 {\n/ /foo\n}", false, []Rule{{FromPath: "/", To: "/foo", Code: 300}}}, | ||||
| 		{"redir 300 {\n/ /foo\n}", false, []Rule{{FromPath: "/", To: "/foo", Code: 300, RequestMatcher: httpserver.IfMatcher{}}}}, | ||||
| 
 | ||||
| 		// test case #1 tests the recognition of an invalid HTTP status code defined outside of block statement | ||||
| 		{"redir 9000 {\n/ /foo\n}", true, []Rule{{}}}, | ||||
| @ -27,22 +28,43 @@ func TestSetup(t *testing.T) { | ||||
| 		{"redir 9000 {\n/ /foo 300\n}", true, []Rule{{}}}, | ||||
| 
 | ||||
| 		// test case #4 tests the recognition of a TO redirection in a block statement.The HTTP status code is set to the default of 301 - MovedPermanently | ||||
| 		{"redir 302 {\n/foo\n}", false, []Rule{{FromPath: "/", To: "/foo", Code: 302}}}, | ||||
| 		{"redir 302 {\n/foo\n}", false, []Rule{{FromPath: "/", To: "/foo", Code: 302, RequestMatcher: httpserver.IfMatcher{}}}}, | ||||
| 
 | ||||
| 		// test case #5 tests the recognition of a TO and From redirection in a block statement | ||||
| 		{"redir {\n/bar /foo 303\n}", false, []Rule{{FromPath: "/bar", To: "/foo", Code: 303}}}, | ||||
| 		{"redir {\n/bar /foo 303\n}", false, []Rule{{FromPath: "/bar", To: "/foo", Code: 303, RequestMatcher: httpserver.IfMatcher{}}}}, | ||||
| 
 | ||||
| 		// test case #6 tests the recognition of a TO redirection in a non-block statement. The HTTP status code is set to the default of 301 - MovedPermanently | ||||
| 		{"redir /foo", false, []Rule{{FromPath: "/", To: "/foo", Code: 301}}}, | ||||
| 		{"redir /foo", false, []Rule{{FromPath: "/", To: "/foo", Code: 301, RequestMatcher: httpserver.IfMatcher{}}}}, | ||||
| 
 | ||||
| 		// test case #7 tests the recognition of a TO and From redirection in a non-block statement | ||||
| 		{"redir /bar /foo 303", false, []Rule{{FromPath: "/bar", To: "/foo", Code: 303}}}, | ||||
| 		{"redir /bar /foo 303", false, []Rule{{FromPath: "/bar", To: "/foo", Code: 303, RequestMatcher: httpserver.IfMatcher{}}}}, | ||||
| 
 | ||||
| 		// test case #8 tests the recognition of multiple redirections | ||||
| 		{"redir {\n / /foo 304 \n} \n redir {\n /bar /foobar 305 \n}", false, []Rule{{FromPath: "/", To: "/foo", Code: 304}, {FromPath: "/bar", To: "/foobar", Code: 305}}}, | ||||
| 		{"redir {\n / /foo 304 \n} \n redir {\n /bar /foobar 305 \n}", false, | ||||
| 			[]Rule{{FromPath: "/", To: "/foo", Code: 304, RequestMatcher: httpserver.IfMatcher{}}, | ||||
| 				{FromPath: "/bar", To: "/foobar", Code: 305, RequestMatcher: httpserver.IfMatcher{}}}}, | ||||
| 
 | ||||
| 		// test case #9 tests the detection of duplicate redirections | ||||
| 		{"redir {\n /bar /foo 304 \n} redir {\n /bar /foo 304 \n}", true, []Rule{{}}}, | ||||
| 
 | ||||
| 		// test case #10 tests the detection of a valid HTTP status code outside of a block statement being overridden by an valid HTTP status code inside statement of a block statement | ||||
| 		{"redir 300 {\n/ /foo 301\n}", false, []Rule{{FromPath: "/", To: "/foo", Code: 301, RequestMatcher: httpserver.IfMatcher{}}}}, | ||||
| 
 | ||||
| 		// test case #11 tests the recognition of a matcher | ||||
| 		{"redir {\n if {port} is 80\n/ /foo\n}", false, []Rule{{FromPath: "/", To: "/foo", Code: 301, | ||||
| 			RequestMatcher: func() httpserver.IfMatcher { | ||||
| 				c := caddy.NewTestController("http", "{\n if {port} is 80\n}") | ||||
| 				matcher, _ := httpserver.SetupIfMatcher(c) | ||||
| 				return matcher.(httpserver.IfMatcher) | ||||
| 			}()}}}, | ||||
| 
 | ||||
| 		// test case #12 tests the detection of a valid HTTP status code outside of a block statement with a matcher | ||||
| 		{"redir 300 {\n if {port} is 80\n/ /foo\n}", false, []Rule{{FromPath: "/", To: "/foo", Code: 300, | ||||
| 			RequestMatcher: func() httpserver.IfMatcher { | ||||
| 				c := caddy.NewTestController("http", "{\n if {port} is 80\n}") | ||||
| 				matcher, _ := httpserver.SetupIfMatcher(c) | ||||
| 				return matcher.(httpserver.IfMatcher) | ||||
| 			}()}}}, | ||||
| 	} { | ||||
| 		c := caddy.NewTestController("http", test.input) | ||||
| 		err := setup(c) | ||||
| @ -64,6 +86,9 @@ func TestSetup(t *testing.T) { | ||||
| 			if recievedRule.Code != test.expectedRules[i].Code { | ||||
| 				t.Errorf("Test case #%d.%d expected a HTTP status code of %d, but recieved a code of %d", j, i, test.expectedRules[i].Code, recievedRule.Code) | ||||
| 			} | ||||
| 			if gotMatcher, expectMatcher := fmt.Sprint(recievedRule.RequestMatcher), fmt.Sprint(test.expectedRules[i].RequestMatcher); gotMatcher != expectMatcher { | ||||
| 				t.Errorf("Test case #%d.%d expected a Matcher %s, but recieved a Matcher %s", j, i, expectMatcher, gotMatcher) | ||||
| 			} | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
|  | ||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user