diff --git a/CHANGELOG.md b/CHANGELOG.md index 705873218..d0a16f8f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Some policies did not have access to the vars exposed when using Liquid (`uri`, `path`, etc.) [PR #891](https://github.com/3scale/apicast/pull/891) - Fix error when loading certain configurations that use OIDC [PR #893](https://github.com/3scale/apicast/pull/893) - Fix error that appeared when combining the liquid context debug policy with policies that contain liquid templates [PR #895](https://github.com/3scale/apicast/pull/895) +- Thread safety issues when rendering Liquid templates [PR #896](https://github.com/3scale/apicast/pull/896) ### Added diff --git a/gateway/src/apicast/template_string.lua b/gateway/src/apicast/template_string.lua index 29092d55f..67060a9c3 100644 --- a/gateway/src/apicast/template_string.lua +++ b/gateway/src/apicast/template_string.lua @@ -1,5 +1,4 @@ local Liquid = require 'liquid' -local LiquidTemplate = Liquid.Template local LiquidInterpreterContext = Liquid.InterpreterContext local LiquidFilterSet = Liquid.FilterSet local LiquidResourceLimit = Liquid.ResourceLimit @@ -68,15 +67,33 @@ end -- Set resource limits to avoid loops local liquid_resource_limit = LiquidResourceLimit:new(nil, nil, 0) +-- TODO: we should move this to liquid-lua and fix it's broken Template interface +local CachedParser = { } +local CachedParser_mt = { __index = CachedParser } + +function CachedParser.new(parser) + local doc = parser:document() + return setmetatable({ doc = doc }, CachedParser_mt) +end + +function CachedParser:document() return self.doc end + +local function liquid_parser(text) + local lexer = Liquid.Lexer:new(text) + local parser = Liquid.Parser:new(lexer) + + return CachedParser.new(parser) +end + function LiquidTemplateString.new(string) - return setmetatable({ template = LiquidTemplate:parse(string) }, + return setmetatable({ parser = liquid_parser(string) }, liquid_template_string_mt) end function LiquidTemplateString:render(context) local available_context = ngx_variable.available_context(context) - return self.template:render( + return Liquid.Interpreter:new(self.parser):interpret( LiquidInterpreterContext:new(available_context), liquid_filter_set, liquid_resource_limit diff --git a/spec/template_string_spec.lua b/spec/template_string_spec.lua index aeaceedd6..e26a0f06e 100644 --- a/spec/template_string_spec.lua +++ b/spec/template_string_spec.lua @@ -33,6 +33,35 @@ describe('template string', function() assert.equals(ngx.md5('something'), liquid_template:render({})) end) + describe(':render', function() + it('it allows context to be garbage collected #gc', function() + local template = TemplateString.new('string', 'liquid') + local context = { 'value' } + + template:render(context) + + -- store the object so we can verify it was GC'd later + local gc = setmetatable({ context = context }, { __mode = 'vk' }) + -- luassert stub takes a reference to parameters passed through + ngx_variable.available_context:revert() + context = nil + + collectgarbage() + assert.is_nil(gc.context) + end) + + it('does not parse the document twice', function() + local template = TemplateString.new('string', 'liquid') + + stub(require('liquid').Parser, 'document', function() + error('this should not be called') + end) + + assert.same('string', template:render({})) + assert.same('string', template:render({})) + end) + end) + describe('.new', function() it('returns nil and an error with invalid type', function() local template, err = TemplateString.new('some_string', 'invalid_type')