Do not update template mtime when there is an error reloading templat…

…es in the render plugin

Previously, if there was an mtime change for the related file, Roda would
update the stored mtime, then try to reload the file.  However, if the file
was invalid (e.g. SyntaxError), then in compiled templates mode, the
compiled template method would not be updated, so it still have the previous
template code associated with it.  If no further changes were made to the
template, future template renders would return the result of the previous
template before the reload, resulting in unexpected behavior until the
template was fixed.

Replace TemplateMtimeWrapper#modified with TemplateMtimeWrapper#if_modified,
which takes a block, and yields if it detects a modification.  The mtime for
the object is not updated unless the block returns normally.  Put the template
reloading code into the block.  For non-compiled templates, attempt to render
the template inside the if_modified block, and only update the mtime if
the rendering succeeds.

This change can result in additional work if the template has not been
fixed after it was broken, since every request after it is broken will
attempt to reload it.  However, template reloading is generally only done
in development mode, and without attempting to reload, the template would
raise an exception just as in previous requests.

While here, remove the unnecessary template argument to compiled_method_lambda,
since it is only ever set to self.

CHANGELOG

@@ -1,5 +1,7 @@
 = master
 
+* Do not update template mtime when there is an error reloading templates in the render plugin (jeremyevans)
+
 * Add hmac_paths plugin for preventing path enumeration and supporting access control (jeremyevans)
 
 = 3.78.0 (2024-03-13)

lib/roda/plugins/render.rb

@@ -335,8 +335,13 @@ def initialize(roda_class, opts, template_opts)
         # If the template file exists and the modification time has
         # changed, rebuild the template file, then call render on it.
         def render(*args, &block)
-          modified?
-          @template.render(*args, &block)
+          res = nil
+          modified = false
+          if_modified do
+            res = @template.render(*args, &block)
+            modified = true
+          end
+          modified ? res : @template.render(*args, &block)
         end
 
         # Return when the template was last modified.  If the template depends on any
@@ -352,20 +357,18 @@ def template_last_modified
 
         # If the template file has been updated, return true and update
         # the template object and the modification time. Other return false.
-        def modified?
+        def if_modified
           begin
             mtime = template_last_modified
           rescue
             # ignore errors
           else
             if mtime != @mtime
-              @mtime = mtime
               reset_template
-              return true
+              yield
+              @mtime = mtime
             end
           end
-
-          false
         end
 
         if COMPILED_METHOD_SUPPORT
@@ -375,13 +378,13 @@ def define_compiled_method(roda_class, method_name, locals_keys=EMPTY_ARRAY)
             mod = roda_class::RodaCompiledTemplates
             internal_method_name = :"_#{method_name}"
             begin
-              mod.send(:define_method, internal_method_name, send(:compiled_method, locals_keys, roda_class))
+              mod.send(:define_method, internal_method_name, compiled_method(locals_keys, roda_class))
             rescue ::NotImplementedError
               return false
             end
 
             mod.send(:private, internal_method_name)
-            mod.send(:define_method, method_name, &compiled_method_lambda(self, roda_class, internal_method_name, locals_keys))
+            mod.send(:define_method, method_name, &compiled_method_lambda(roda_class, internal_method_name, locals_keys))
             mod.send(:private, method_name)
 
             method_name
@@ -397,10 +400,11 @@ def compiled_method(locals_keys=EMPTY_ARRAY, roda_class=nil)
           # Return the lambda used to define the compiled template method.  This
           # is separated into its own method so the lambda does not capture any
           # unnecessary local variables
-          def compiled_method_lambda(template, roda_class, method_name, locals_keys=EMPTY_ARRAY)
+          def compiled_method_lambda(roda_class, method_name, locals_keys=EMPTY_ARRAY)
             mod = roda_class::RodaCompiledTemplates
+            template = self
             lambda do |locals, &block|
-              if template.modified?
+              template.if_modified do
                 mod.send(:define_method, method_name, Render.tilt_template_compiled_method(template, locals_keys, roda_class))
                 mod.send(:private, method_name)
               end

spec/plugin/render_spec.rb

@@ -138,6 +138,30 @@
     end
   end
 
+  it "does not update mtime if there was an error rebuilding the template" do
+    app(:bare) do
+      plugin :render, :views=>"./spec", :cache=>false
+
+      route do |r|
+        @a = 'a'
+        render(iv)
+      end
+    end
+
+    t = Time.now
+    body.strip.must_equal "a"
+
+    content = File.binread(file)
+    File.binwrite(file, content + "<% end %>")
+    File.utime(t, t+1, file)
+    proc{body}.must_raise SyntaxError
+    proc{body}.must_raise SyntaxError
+
+    File.binwrite(file, content + "b")
+    File.utime(t, t+1, file)
+    body.gsub("\n", '').must_equal "ab"
+  end
+
   it "does not check mtime if :cache render option is used" do
     app(:bare) do
       plugin :render, :views=>"./spec", :cache=>false