From 8c3e443de9d26d0e722956107db8a36134c0134c Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 30 Jun 2016 15:07:28 -0700 Subject: [PATCH 1/5] First attempt at mitigating #3206 --- certbot-nginx/certbot_nginx/nginxparser.py | 31 +++++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py index d6c352296..895c4f8a3 100644 --- a/certbot-nginx/certbot_nginx/nginxparser.py +++ b/certbot-nginx/certbot_nginx/nginxparser.py @@ -38,19 +38,42 @@ class RawNginxParser(object): assignment = space + key + Optional(space + value, default=None) + semicolon location_statement = space + Optional(modifier) + Optional(space + location + space) if_statement = space + Literal("if") + space + condition + space + map_statement = space + Literal("map") + space + nonspace + space + dollar_var + space + + # This is NOT an accurate way to parse nginx map entries; it's almost + # certianly too permissive and may be wrong in other ways, but it should + # preserve things correctly in mmmmost or all cases. + # - it sometimes splits the two tokens incorrectly eg + # '''"~Opera Mini" 1''' -> ['"~Opera', ' Mini" 1'] + # - I can neither prove nor disprove that it is corect wrt all escaped + # semicolon situations + # Addresses https://github.com/fatiherikli/nginxparser/issues/19 + + map_entry = space + nonspace + value + space + semicolon + map_block = Forward() + map_block << Group( + # key could for instance be "server" or "http", or "location" (in which case + # location_statement needs to have a non-empty location) + Group(map_statement).leaveWhitespace() + + left_bracket + + Group(ZeroOrMore(Group(comment | map_entry)) + space).leaveWhitespace() + + right_bracket) + + block = Forward() block << Group( # key could for instance be "server" or "http", or "location" (in which case # location_statement needs to have a non-empty location) - (Group(space + key + location_statement) ^ Group(if_statement) ^ - Group(map_statement)).leaveWhitespace() + + (Group(space + key + location_statement) ^ Group(if_statement)).leaveWhitespace() + left_bracket + - Group(ZeroOrMore(Group(comment | assignment) | block) + space).leaveWhitespace() + + Group(ZeroOrMore(Group(comment | assignment) | block | map_block) + space).leaveWhitespace() + right_bracket) - script = OneOrMore(Group(comment | assignment) ^ block) + space + stringEnd + + + script = OneOrMore(Group(comment | assignment) ^ block ^ map_block) + space + stringEnd script.parseWithTabs() def __init__(self, source): From db8ddac4e252780fb4015298b7cbe0388d340f10 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 30 Jun 2016 15:13:35 -0700 Subject: [PATCH 2/5] lint & tweak --- certbot-nginx/certbot_nginx/nginxparser.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py index 895c4f8a3..50ed41eb9 100644 --- a/certbot-nginx/certbot_nginx/nginxparser.py +++ b/certbot-nginx/certbot_nginx/nginxparser.py @@ -40,7 +40,6 @@ class RawNginxParser(object): if_statement = space + Literal("if") + space + condition + space map_statement = space + Literal("map") + space + nonspace + space + dollar_var + space - # This is NOT an accurate way to parse nginx map entries; it's almost # certianly too permissive and may be wrong in other ways, but it should # preserve things correctly in mmmmost or all cases. @@ -49,7 +48,6 @@ class RawNginxParser(object): # - I can neither prove nor disprove that it is corect wrt all escaped # semicolon situations # Addresses https://github.com/fatiherikli/nginxparser/issues/19 - map_entry = space + nonspace + value + space + semicolon map_block = Forward() map_block << Group( @@ -60,18 +58,14 @@ class RawNginxParser(object): Group(ZeroOrMore(Group(comment | map_entry)) + space).leaveWhitespace() + right_bracket) - block = Forward() - block << Group( # key could for instance be "server" or "http", or "location" (in which case # location_statement needs to have a non-empty location) (Group(space + key + location_statement) ^ Group(if_statement)).leaveWhitespace() + left_bracket + - Group(ZeroOrMore(Group(comment | assignment) | block | map_block) + space).leaveWhitespace() + - right_bracket) - - + Group(ZeroOrMore(Group(comment | assignment) | block | map_block) + space).leaveWhitespace() + + right_bracket) script = OneOrMore(Group(comment | assignment) ^ block ^ map_block) + space + stringEnd script.parseWithTabs() From be8f0bc53b657fdf3a104c355858c396f96eb6a0 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 30 Jun 2016 15:29:38 -0700 Subject: [PATCH 3/5] Do a better job of parsing map patterns --- certbot-nginx/certbot_nginx/nginxparser.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py index 50ed41eb9..0cc912515 100644 --- a/certbot-nginx/certbot_nginx/nginxparser.py +++ b/certbot-nginx/certbot_nginx/nginxparser.py @@ -27,6 +27,8 @@ class RawNginxParser(object): condition = Regex(r"\(.+\)") # Matches anything that is not a special character AND any chars in single # or double quotes + # All of these COULD be upgraded to something like + # https://stackoverflow.com/a/16130746 value = Regex(r"((\".*\")?(\'.*\')?[^\{\};,]?)+") location = CharsNotIn("{};," + string.whitespace) # modifier for location uri [ = | ~ | ~* | ^~ ] @@ -43,14 +45,13 @@ class RawNginxParser(object): # This is NOT an accurate way to parse nginx map entries; it's almost # certianly too permissive and may be wrong in other ways, but it should # preserve things correctly in mmmmost or all cases. - # - it sometimes splits the two tokens incorrectly eg - # '''"~Opera Mini" 1''' -> ['"~Opera', ' Mini" 1'] + # # - I can neither prove nor disprove that it is corect wrt all escaped # semicolon situations # Addresses https://github.com/fatiherikli/nginxparser/issues/19 - map_entry = space + nonspace + value + space + semicolon - map_block = Forward() - map_block << Group( + map_pattern = Regex(r'".*"') | Regex(r"'.*'") | nonspace + map_entry = space + map_pattern + space + value + space + semicolon + map_block = Group( # key could for instance be "server" or "http", or "location" (in which case # location_statement needs to have a non-empty location) Group(map_statement).leaveWhitespace() + From 68500cd4361bd0d07167c8f42a77adec3ac034f9 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 9 Jul 2016 15:13:09 -0700 Subject: [PATCH 4/5] Don't allow dollar_var to swalllow characters like "{" --- certbot-nginx/certbot_nginx/nginxparser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py index 0cc912515..1859777d8 100644 --- a/certbot-nginx/certbot_nginx/nginxparser.py +++ b/certbot-nginx/certbot_nginx/nginxparser.py @@ -23,7 +23,7 @@ class RawNginxParser(object): right_bracket = space.leaveWhitespace() + Literal("}").suppress() semicolon = Literal(";").suppress() key = Word(alphanums + "_/+-.") - dollar_var = Combine(Literal('$') + nonspace) + dollar_var = Combine(Literal('$') + Regex(r"[^\{\};,\s]+")) condition = Regex(r"\(.+\)") # Matches anything that is not a special character AND any chars in single # or double quotes From 557d2e80d37ac61258f62a4dd7e7fc1eb9d1ca29 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 13 Jul 2016 15:37:44 -0700 Subject: [PATCH 5/5] Test case from https://github.com/certbot/certbot/pull/3230#issuecomment-231546594 --- .../nginx/nginx-roundtrip-testdata/anothermapcase/nginx.conf | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 certbot-compatibility-test/nginx/nginx-roundtrip-testdata/anothermapcase/nginx.conf diff --git a/certbot-compatibility-test/nginx/nginx-roundtrip-testdata/anothermapcase/nginx.conf b/certbot-compatibility-test/nginx/nginx-roundtrip-testdata/anothermapcase/nginx.conf new file mode 100644 index 000000000..b3ca02f92 --- /dev/null +++ b/certbot-compatibility-test/nginx/nginx-roundtrip-testdata/anothermapcase/nginx.conf @@ -0,0 +1,3 @@ +map $uri $blogname{ + ~^(?P/[^/]+/)files/(.*) $blogpath ; +}