Further ESI regex robustness plus tests.

ledgetech · Nov 11, 2015 · 07c15d8 · 07c15d8
1 parent f39ba97
commit 07c15d8
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 19 deletions.
diff --git a/lib/ledge/esi.lua b/lib/ledge/esi.lua
@@ -17,6 +17,7 @@ local ngx_crc32_long = ngx.crc32_long
 local ngx_var = ngx.var
 local ngx_log = ngx.log
 local ngx_ERR = ngx.ERR
+local ngx_INFO = ngx.INFO
 local tbl_concat = table.concat
 local co_yield = coroutine.yield
 local co_create = coroutine.create
@@ -66,7 +67,7 @@ local lua_reserved_words =  "and|break|false|true|function|for|repeat|while|do|e
                             "local|nil|not|or|return|then|until|else|elseif"
 
 -- $1: Any lua reserved words not found within quotation marks
-local esi_non_quoted_lua_words =    [[(?!\'{1}|\"{1})(?:.*)(\b]] .. lua_reserved_words ..
+local esi_non_quoted_lua_words =    [[(?!\'{1}|\"{1})(?:.*?)(\b]] .. lua_reserved_words ..
                                     [[\b)(?!\'{1}|\"{1})]]
 
 
@@ -140,7 +141,7 @@ end
 
 -- Used in esi_replace_vars. Declared locally to avoid runtime closure definition.
 local function _esi_gsub_in_vars_tags(m)
-    return ngx_re_gsub(m[1], esi_var_pattern, esi_eval_var, "soj")
+    return m[1] .. ngx_re_gsub(m[2], esi_var_pattern, esi_eval_var, "soj") .. m[3]
 end
 
 
@@ -171,7 +172,11 @@ end
 local function _esi_evaluate_condition(condition)
     -- Remove lua reserved words (if / then / repeat / function etc)
     -- which are not quoted as strings
-    condition = ngx_re_gsub(condition, esi_non_quoted_lua_words, "", "oj")
+    local reps
+    condition, reps = ngx_re_gsub(condition, esi_non_quoted_lua_words, "", "oj")
+    if reps > 0 then
+        ngx_log(ngx_INFO, "Removed " .. reps .. " unquoted Lua reserved words")
+    end
 
     -- Replace ESI operand syntax with Lua equivalents.
     local op_replacements = {
@@ -246,13 +251,13 @@ local function esi_replace_vars(chunk)
     )
 
     -- For every esi:vars block, substitute any number of variables found.
-    chunk = ngx_re_gsub(chunk, "<esi:vars>(.*)</esi:vars>", _esi_gsub_in_vars_tags, "soj")
+    chunk = ngx_re_gsub(chunk, "(<esi:[^>]+>)(.+?)(</esi:[^>]+>)", _esi_gsub_in_vars_tags, "soj")
 
     -- Remove vars tags that are left over
     chunk = ngx_re_gsub(chunk, "(<esi:vars>|</esi:vars>)", "", "soj")
 
     -- Replace vars inline in any other esi: tags, retaining the surrounding tags.
-    chunk = ngx_re_gsub(chunk, "(<esi:)(.+)(.*>)", _esi_gsub_vars_in_other_tags, "oj")
+    chunk = ngx_re_gsub(chunk, [[(<esi:)([^>]+)([/\s]*>)]], _esi_gsub_vars_in_other_tags, "oj")
 
     return chunk
 end
@@ -268,7 +273,7 @@ local function esi_fetch_include(include_tag, buffer_size, pre_include_callback,
 
     local src, err = ngx_re_match(
         include_tag,
-        "src=\"(.+)\".*/>",
+        "src=\"(.+)\"[^>*]/>",
         "oj"
     )
 
@@ -483,11 +488,11 @@ function _M.get_process_filter(reader, pre_include_callback, recursion_limit)
             local escaped = 0
             if chunk then
                 if has_esi then
-                    -- Remove <!--esi--> 
+                    -- Remove <!--esi-->
                     chunk, escaped = ngx_re_gsub(chunk, "(<!--esi(.*?)-->)", "$2", "soj")
 
                     -- Remove comments.
-                    chunk = ngx_re_gsub(chunk, "<esi:comment (?:.*)/>", "", "soj")
+                    chunk = ngx_re_gsub(chunk, "<esi:comment (?:.*?)/>", "", "soj")
 
                     -- Remove 'remove' blocks
                     chunk = ngx_re_gsub(chunk, "(<esi:remove>.*?</esi:remove>)", "", "soj")

diff --git a/t/10-esi.t b/t/10-esi.t
@@ -1,7 +1,7 @@
 use Test::Nginx::Socket;
 use Cwd qw(cwd);
 
-plan tests =>  repeat_each() * (blocks() * 3) + 5;
+plan tests =>  repeat_each() * (blocks() * 3) + 6;
 
 my $pwd = cwd();
 
@@ -536,7 +536,7 @@ location /esi_9 {
         ngx.say("HTTP_COOKIE: $(HTTP_COOKIE)");
         ngx.say("HTTP_COOKIE{SQ_SYSTEM_SESSION}: $(HTTP_COOKIE{SQ_SYSTEM_SESSION})");
         ngx.say("</esi:vars>");
-        ngx.say("<esi:vars>$(HTTP_COOKIE{SQ_SYSTEM_SESSION})</esi:vars>OK<esi:vars>$(HTTP_COOKIE{SQ_SYSTEM_SESSION})</esi:vars>")
+        ngx.say("<esi:vars>$(HTTP_COOKIE{SQ_SYSTEM_SESSION})</esi:vars>$(HTTP_COOKIE)<esi:vars>$(QUERY_STRING)</esi:vars>")
     ';
 }
 --- more_headers
@@ -551,7 +551,7 @@ HTTP_COOKIE{SQ_SYSTEM_SESSION}: hello
 HTTP_COOKIE: myvar=foo; SQ_SYSTEM_SESSION=hello
 HTTP_COOKIE{SQ_SYSTEM_SESSION}: hello
 
-helloOKhello
+hello$(HTTP_COOKIE)t=1
 
 
 === TEST 9b: Multiple Variable evaluation
@@ -564,7 +564,7 @@ location /esi_9b_prx {
 location /esi_9b {
     default_type text/html;
     content_by_lua '
-        ngx.say("<esi:include src=\\"/fragment1b?$(QUERY_STRING)&test=$(HTTP_X_ESI_TEST)\\" />")
+        ngx.say("<esi:include src=\\"/fragment1b?$(QUERY_STRING)&test=$(HTTP_X_ESI_TEST)\\" /> <a href=\\"$(QUERY_STRING)\\" />")
     ';
 }
 location /fragment1b {
@@ -578,7 +578,7 @@ GET /esi_9b_prx?t=1
 X-ESI-Test: foobar
 --- raw_response_headers_unlike: Surrogate-Control: content="ESI/1.0\"\r\n
 --- response_body
-FRAGMENT:t=1&test=foobar
+FRAGMENT:t=1&test=foobar <a href="$(QUERY_STRING)" />
 
 
 
@@ -1140,7 +1140,7 @@ location /esi_14_prx {
 }
 location /esi_14 {
     default_type text/html;
-    content_by_lua '
+content_by_lua '
 local content = [[Hello
 <esi:choose>
 <esi:when test="$(QUERY_STRING{a}) == 1">
@@ -1154,8 +1154,8 @@ Will never happen
 </esi:otherwise>
 </esi:choose>
 Goodbye]]
-        ngx.say(content)
-    ';
+    ngx.say(content)
+';
 }
 --- request
 GET /esi_14_prx?a=1
@@ -1328,13 +1328,14 @@ location /esi_17 {
             "\\\'hello\\\' == \\\'hello\\\'",
             "\\\'hello\\\' != \\\'goodbye\\\'",
             "\\\'repeat\\\' != \\\'function\\\'", -- use of lua words in strings
+            "\\\'repeat\\\' != function", -- use of lua words unquoted
             "\\\' repeat sentence with function in it \\\' == \\\' repeat sentence with function in it \\\'", -- use of lua words in strings
             [["this string has \\\"quotes\\\"" == "this string has \\\"quotes\\\""]],
             "$(QUERY_STRING{msg}) == \\\'hello\\\'",
         }
 
         for _,c in ipairs(conditions) do
-            ngx.say([[<esi:choose><esi:when test="]], c, [[">]], c, 
+            ngx.say([[<esi:choose><esi:when test="]], c, [[">]], c,
                     [[</esi:when><esi:otherwise>Failed</esi:otherwise></esi:choose>]])
             ngx.say("")
         end
@@ -1355,11 +1356,44 @@ GET /esi_17_prx?msg=hello
 'hello' == 'hello'
 'hello' != 'goodbye'
 'repeat' != 'function'
+Failed
 ' repeat sentence with function in it ' == ' repeat sentence with function in it '
 "this string has \"quotes\"" == "this string has \"quotes\""
 hello == 'hello'
 
 
+=== TEST 17b: Unquoted Lua reserved words in conditions removed
+--- http_config eval: $::HttpConfig
+--- config
+location /esi_17b_prx {
+    rewrite ^(.*)_prx$ $1 break;
+    content_by_lua 'run()';
+}
+location /esi_17b {
+    default_type text/html;
+    content_by_lua '
+        local content = [[
+<esi:choose>
+<esi:when test="function function == function function">
+OK
+</esi:when>
+<esi:otherwise>
+Otherwise
+</esi:otherwise>
+</esi:choose>
+]]
+        ngx.print(content)
+    ';
+}
+--- request
+GET /esi_17b_prx
+--- raw_response_headers_unlike: Surrogate-Control: content="ESI/1.0\"\r\n
+--- response_body
+Otherwise
+--- error_log
+Removed 4 unquoted Lua reserved words
+
+
 === TEST 18: Surrogate-Control with lower version number still works.
 --- http_config eval: $::HttpConfig
 --- config
@@ -1470,13 +1504,13 @@ location /esi_22_prx {
 location /esi_22 {
     default_type text/html;
     content_by_lua '
-        ngx.print([[1234<esi:comment text="comment text" />]])
+        ngx.print([[1234<esi:comment text="comment text" /> 5678<esi:comment text="comment text 2" />]])
     ';
 }
 --- request
 GET /esi_22_prx
 --- raw_response_headers_unlike: Surrogate-Control: content="ESI/1.0\"\r\n
---- response_body: 1234
+--- response_body: 1234 5678
 
 
 === TEST 23a: Surrogate-Control removed when ESI enabled but no work needed (slow path)