Skip to content

Commit 394edbf

Browse files
committed
Fix hermetic recorder host integration edge cases
Keep recorder hooks inert for host contexts without Liquid registers, capture only bytes produced by the template, and exclude memory-limited renders from the corpus. Strict replay now stubs application-only inline and block tags from the recorded sources. Assisted-By: devx/948e6cb4-458b-480b-8d99-d726473b381b
1 parent 42a1ffe commit 394edbf

5 files changed

Lines changed: 134 additions & 4 deletions

File tree

lib/liquid/drop.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,12 @@ def invoke_drop(method_or_key)
4343
liquid_method_missing(method_or_key)
4444
end
4545

46-
recorder = @context&.registers&.[](TemplateRecorder::REGISTER_KEY) if defined?(TemplateRecorder)
46+
# A host application may assign its own object as a drop's context, and a
47+
# drop can be invoked outside any render. Neither case has registers, and
48+
# instrumentation must never turn either into a NoMethodError.
49+
if defined?(TemplateRecorder) && @context.respond_to?(:registers)
50+
recorder = @context.registers[TemplateRecorder::REGISTER_KEY]
51+
end
4752
recorder&.emit_drop_read(self, method_or_key, result)
4853
result
4954
end

lib/liquid/strainer_template.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,11 @@ def invoke(method, *args)
5656
args.first
5757
end
5858

59-
recorder = @context&.registers&.[](TemplateRecorder::REGISTER_KEY) if defined?(TemplateRecorder)
59+
# See Drop#invoke_drop: the context is not guaranteed to be a
60+
# Liquid::Context, and instrumentation must not raise.
61+
if defined?(TemplateRecorder) && @context.respond_to?(:registers)
62+
recorder = @context.registers[TemplateRecorder::REGISTER_KEY]
63+
end
6064
recorder&.emit_filter_call(method, args.first, args.drop(1), result)
6165
result
6266
rescue ::ArgumentError => e

lib/liquid/template.rb

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,14 +204,23 @@ def render(*args)
204204

205205
rendered_output = nil
206206
render_succeeded = false
207+
# A caller may hand in a buffer that already holds bytes this template did
208+
# not produce, and may keep appending to it after this render returns. The
209+
# recording must describe only what the template rendered, so remember
210+
# where its output starts.
211+
recorded_output_start = recording ? (output || '').bytesize : 0
207212
begin
208213
# render the nodelist.
209214
rendered_output = @root.render_to_output_buffer(context, output || +'')
210215
render_succeeded = true
211216
rendered_output
212217
rescue Liquid::MemoryError => e
213218
rendered_output = context.handle_error(e)
214-
render_succeeded = true
219+
# The caller still gets the error text, but a render that hit the memory
220+
# limit is truncated by definition: recording it as a success would put
221+
# a partial render into the corpus as if it were the specified output.
222+
render_succeeded = false
223+
recorded_output_start = 0
215224
rendered_output
216225
ensure
217226
if previous_error_mode
@@ -225,7 +234,15 @@ def render(*args)
225234
else
226235
recorder_registers.delete(TemplateRecorder::REGISTER_KEY)
227236
end
228-
recording_session.finish_render(recording, rendered_output, context, success: render_succeeded)
237+
# Slice off any caller-supplied prefix, and copy: the buffer is the
238+
# caller's and may still be appended to, which would otherwise leak
239+
# bytes into a recording that has already been taken.
240+
recorded_output = if rendered_output.is_a?(String)
241+
rendered_output.byteslice(recorded_output_start, rendered_output.bytesize - recorded_output_start).dup
242+
else
243+
rendered_output
244+
end
245+
recording_session.finish_render(recording, recorded_output, context, success: render_succeeded)
229246
end
230247
@errors = context.errors
231248
end

lib/liquid/template_recorder.rb

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ class TemplateRecorder
1717
REGISTER_KEY = :__liquid_template_recorder
1818
REPLAYER_REGISTER_KEY = :__liquid_template_recorder_replayer
1919

20+
# Opening tag markup, with or without whitespace control.
21+
TAG_NAME_PATTERN = /\{%-?\s*(\w+)/
22+
2023
class Error < StandardError; end
2124
class ReplayError < Error; end
2225
class SerializationError < Error; end
@@ -509,11 +512,58 @@ def strict_environment
509512
tags&.each do |name, tag_class|
510513
tags[name] = replay_tag_class(tag_class, name)
511514
end
515+
register_host_tag_stubs(tags)
512516
Liquid::Environment.build(tags: tags) do |environment|
513517
environment.strainer_template = strainer
514518
end
515519
end
516520

521+
# Registers a stub for every tag the recorded sources mention that this
522+
# environment does not know.
523+
#
524+
# A recording taken inside a host application names that application's
525+
# tags, and strict replay re-parses the recorded source. Without a stub
526+
# the parse fails and the recording is replayable only inside the
527+
# application it came from, which defeats the point of recording it.
528+
#
529+
# A stub is only ever parsed. A tag that was invoked has its recorded
530+
# output substituted by +replay_tag_class+; a block stub consumes its
531+
# body without rendering it, so a custom tag nested inside another one
532+
# never executes and never consumes a recorded call.
533+
def register_host_tag_stubs(tags)
534+
names, blocks = host_tag_names(tags)
535+
536+
names.each do |name|
537+
stub = blocks.include?(name) ? Liquid::Block : Liquid::Tag
538+
tags[name] = replay_tag_class(Class.new(stub), name)
539+
end
540+
end
541+
542+
# Unknown tag names in every recorded source, and which of them are
543+
# closed by a matching +end+ tag and so must parse as blocks.
544+
def host_tag_names(known)
545+
names = []
546+
blocks = []
547+
548+
recorded_sources.each do |source|
549+
source.scan(TAG_NAME_PATTERN) do |(name)|
550+
if name.start_with?("end")
551+
blocks << name.delete_prefix("end")
552+
elsif !known.key?(name) && !names.include?(name)
553+
names << name
554+
end
555+
end
556+
end
557+
558+
[names, blocks]
559+
end
560+
561+
def recorded_sources
562+
sources = Array(@record["templates"]).filter_map { |template| template["source"] }
563+
sources.concat(Array(@record["file_system"]&.values))
564+
sources.compact
565+
end
566+
517567
def replay_tag_class(tag_class, name)
518568
replayer = self
519569
Class.new(tag_class) do

test/integration/template_recorder_test.rb

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,17 @@ def test_render_failure_is_not_written_to_jsonl
160160
refute_path_exists(recording)
161161
end
162162

163+
def test_memory_limited_render_is_not_recorded_as_successful
164+
writer = CollectingWriter.new
165+
template = Liquid::Template.parse("0123456789")
166+
template.resource_limits.render_length_limit = 9
167+
168+
output = Liquid::TemplateRecorder.record(writer) { template.render }
169+
170+
assert_equal("Liquid error: Memory limits exceeded", output)
171+
assert_empty(writer.records)
172+
end
173+
163174
def test_tampered_template_is_rejected
164175
Liquid::TemplateRecorder.record(path) { Liquid::Template.parse("safe").render! }
165176
session = JSON.parse(File.read(path))
@@ -311,6 +322,35 @@ def test_unsupported_accessed_values_do_not_affect_the_render
311322
assert_equal({}, writer.records.first["assigns"])
312323
end
313324

325+
def test_recorder_hooks_accept_host_application_contexts
326+
product = ProductDrop.new("Computed", "secret")
327+
product.context = Object.new
328+
assert_equal("Computed", product.invoke_drop("title"))
329+
330+
filter = Module.new do
331+
def passthrough(input)
332+
input
333+
end
334+
end
335+
strainer_class = Class.new(Liquid::StrainerTemplate)
336+
strainer_class.add_filter(filter)
337+
338+
assert_equal("value", strainer_class.new(Object.new).invoke(:passthrough, "value"))
339+
end
340+
341+
def test_records_only_output_produced_by_the_template
342+
writer = CollectingWriter.new
343+
output = +"prefix:"
344+
345+
Liquid::TemplateRecorder.record(writer) do
346+
Liquid::Template.parse("body").render!({}, output: output)
347+
output << ":suffix"
348+
end
349+
350+
assert_equal("prefix:body:suffix", output)
351+
assert_equal("body", writer.records.first["output"])
352+
end
353+
314354
def test_strict_replay_accepts_the_application_environment
315355
environment = Liquid::Environment.build(
316356
tags: Liquid::Environment.default.tags.merge("marker" => MarkerTag),
@@ -329,6 +369,20 @@ def test_strict_replay_accepts_the_application_environment
329369
assert_equal("custom", replay.render)
330370
end
331371

372+
def test_strict_replay_stubs_host_application_tags
373+
environment = Liquid::Environment.build(
374+
tags: Liquid::Environment.default.tags.merge("wrapper" => WrapperTag, "marker" => MarkerTag),
375+
)
376+
writer = CollectingWriter.new
377+
Liquid::TemplateRecorder.record(writer) do
378+
Liquid::Template.parse("{% wrapper %}{% marker %}{% endwrapper %}", environment: environment).render!
379+
end
380+
381+
replay = Liquid::TemplateRecorder::Replayer.new(writer.records.first, mode: :strict)
382+
383+
assert_equal("custom", replay.render)
384+
end
385+
332386
def test_captures_file_reads_that_happen_before_the_template_starts_rendering
333387
writer = CollectingWriter.new
334388
Liquid::TemplateRecorder.record(writer) do

0 commit comments

Comments
 (0)