From 79f50b0568be8c5c08144f058f158193e48ec4a9 Mon Sep 17 00:00:00 2001 From: Anatol Broder Date: Mon, 16 Sep 2013 08:15:10 +0200 Subject: [PATCH 01/13] Resolve conflict --- lib/jekyll/tags/include.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index 0184a6a4..b41c03bc 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -55,7 +55,7 @@ eos return error end - source = File.read(File.join(includes_dir, @file)) + source = read_file(File.join(includes_dir, @file)) partial = Liquid::Template.parse(source) context.stack do @@ -77,13 +77,17 @@ eos if !File.exists?(file) return "Included file #{@file} not found in _includes directory" elsif File.symlink?(file) - return "The included file '_includes/#{@file}' should not be a symlink" + return "The included file '_includes/#{file}' should not be a symlink" end end def blank? false end + + def read_file(file) + return File.read(file) + end end end end From 081b974114ca2912866e36d45a77ff4f8150503e Mon Sep 17 00:00:00 2001 From: Anatol Broder Date: Mon, 16 Sep 2013 08:19:56 +0200 Subject: [PATCH 02/13] Resolve conflict --- lib/jekyll/tags/include.rb | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index b41c03bc..3cb8820b 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -4,6 +4,8 @@ module Jekyll MATCHER = /([\w-]+)\s*=\s*(?:"([^"\\]*(?:\\.[^"\\]*)*)"|'([^'\\]*(?:\\.[^'\\]*)*)'|([\w\.-]+))/ + INCLUDES_DIR = '_includes' + def initialize(tag_name, markup, tokens) super @file, @params = markup.strip.split(' ', 2); @@ -49,14 +51,13 @@ eos end def render(context) - includes_dir = File.join(context.registers[:site].source, '_includes') + dir = includes_dir(context) - if error = validate_file(includes_dir) + if error = validate_file(dir) return error end - source = read_file(File.join(includes_dir, @file)) - partial = Liquid::Template.parse(source) + partial = Liquid::Template.parse(source(dir)) context.stack do context['include'] = parse_params(context) if @params @@ -64,20 +65,20 @@ eos end end - def validate_file(includes_dir) - if File.symlink?(includes_dir) - return "Includes directory '#{includes_dir}' cannot be a symlink" + def validate_file(dir) + if File.symlink?(dir) + return "Includes directory '#{dir}' cannot be a symlink" end if @file !~ /^[a-zA-Z0-9_\/\.-]+$/ || @file =~ /\.\// || @file =~ /\/\./ return "Include file '#{@file}' contains invalid characters or sequences" end - file = File.join(includes_dir, @file) + file = File.join(dir, @file) if !File.exists?(file) - return "Included file #{@file} not found in _includes directory" + return "Included file #{@file} not found in #{INCLUDES_DIR} directory" elsif File.symlink?(file) - return "The included file '_includes/#{file}' should not be a symlink" + return "The included file '#{INCLUDES_DIR}/#{@file}' should not be a symlink" end end @@ -85,8 +86,14 @@ eos false end - def read_file(file) - return File.read(file) + # This method allows to modify the file content by inheriting from the class. + # Don’t refactor it. + def source(dir) + File.read(File.join(dir, @file)) + end + + def includes_dir(context) + File.join(context.registers[:site].source, INCLUDES_DIR) end end end From 891ea8f604d6fb17d8d9b61a7ae524965ee7469e Mon Sep 17 00:00:00 2001 From: Anatol Broder Date: Mon, 16 Sep 2013 08:37:07 +0200 Subject: [PATCH 03/13] People can symlink it if they want in unsafe mode --- lib/jekyll/tags/include.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index 3cb8820b..4bae7db1 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -66,7 +66,7 @@ eos end def validate_file(dir) - if File.symlink?(dir) + if File.symlink?(dir) && context.registers[:site].safe? return "Includes directory '#{dir}' cannot be a symlink" end @@ -77,7 +77,7 @@ eos file = File.join(dir, @file) if !File.exists?(file) return "Included file #{@file} not found in #{INCLUDES_DIR} directory" - elsif File.symlink?(file) + elsif File.symlink?(file) && context.registers[:site].safe? return "The included file '#{INCLUDES_DIR}/#{@file}' should not be a symlink" end end From 3dadc94ce45ef5ef8bc492b1475cb4a460e39cd6 Mon Sep 17 00:00:00 2001 From: Anatol Broder Date: Mon, 16 Sep 2013 23:16:41 +0200 Subject: [PATCH 04/13] =?UTF-8?q?Don=E2=80=99t=20repeat=20yourself?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/jekyll/tags/include.rb | 59 +++++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 20 deletions(-) diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index 4bae7db1..c0d3be06 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -4,6 +4,8 @@ module Jekyll MATCHER = /([\w-]+)\s*=\s*(?:"([^"\\]*(?:\\.[^"\\]*)*)"|'([^'\\]*(?:\\.[^'\\]*)*)'|([\w\.-]+))/ + VALID_SYNTAX = "{% include file.ext param='value' param2='value' %}" + INCLUDES_DIR = '_includes' def initialize(tag_name, markup, tokens) @@ -35,6 +37,26 @@ module Jekyll # ensure the entire markup string from start to end is valid syntax, and params are separated by spaces def validate_syntax + validate_file_name + validate_params + end + + def validate_file_name + if @file !~ /^[a-zA-Z0-9_\/\.-]+$/ || @file =~ /\.\// || @file =~ /\/\./ + raise SyntaxError.new <<-eos +Invalid syntax for include tag. File contains invalid characters or sequences: + + #{@file} + +Valid syntax: + + #{VALID_SYNTAX} + +eos + end + end + + def validate_params full_matcher = Regexp.compile('\A\s*(?:' + MATCHER.to_s + '(?=\s|\z)\s*)*\z') unless @params =~ full_matcher raise SyntaxError.new <<-eos @@ -44,20 +66,24 @@ Invalid syntax for include tag: Valid syntax: - {% include file.ext param='value' param2="value" %} + #{VALID_SYNTAX} eos end end def render(context) - dir = includes_dir(context) - - if error = validate_file(dir) + dir = File.join(context.registers[:site].source, INCLUDES_DIR) + if error = validate_dir(dir, context.registers[:site].safe) return error end - partial = Liquid::Template.parse(source(dir)) + file = File.join(dir, @file) + if error = validate_file(dir, context.registers[:site].safe) + return error + end + + partial = Liquid::Template.parse(source(file)) context.stack do context['include'] = parse_params(context) if @params @@ -65,19 +91,16 @@ eos end end - def validate_file(dir) - if File.symlink?(dir) && context.registers[:site].safe? + def validate_dir(dir, safe) + if File.symlink?(dir) && safe? return "Includes directory '#{dir}' cannot be a symlink" end + end - if @file !~ /^[a-zA-Z0-9_\/\.-]+$/ || @file =~ /\.\// || @file =~ /\/\./ - return "Include file '#{@file}' contains invalid characters or sequences" - end - - file = File.join(dir, @file) + def validate_file(file, safe) if !File.exists?(file) - return "Included file #{@file} not found in #{INCLUDES_DIR} directory" - elsif File.symlink?(file) && context.registers[:site].safe? + return "Included file '#{@file}' not found in '#{INCLUDES_DIR}' directory" + elsif File.symlink?(file) && safe? return "The included file '#{INCLUDES_DIR}/#{@file}' should not be a symlink" end end @@ -88,12 +111,8 @@ eos # This method allows to modify the file content by inheriting from the class. # Don’t refactor it. - def source(dir) - File.read(File.join(dir, @file)) - end - - def includes_dir(context) - File.join(context.registers[:site].source, INCLUDES_DIR) + def source(file) + File.read(file) end end end From a42e57f274696e7c76addefab7fbd96704cd7084 Mon Sep 17 00:00:00 2001 From: Anatol Broder Date: Tue, 17 Sep 2013 20:14:41 +0200 Subject: [PATCH 05/13] Rename constants --- lib/jekyll/tags/include.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index c0d3be06..89aad0ea 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -2,9 +2,9 @@ module Jekyll module Tags class IncludeTag < Liquid::Tag - MATCHER = /([\w-]+)\s*=\s*(?:"([^"\\]*(?:\\.[^"\\]*)*)"|'([^'\\]*(?:\\.[^'\\]*)*)'|([\w\.-]+))/ + VALID_SYNTAX = /([\w-]+)\s*=\s*(?:"([^"\\]*(?:\\.[^"\\]*)*)"|'([^'\\]*(?:\\.[^'\\]*)*)'|([\w\.-]+))/ - VALID_SYNTAX = "{% include file.ext param='value' param2='value' %}" + SYNTAX_EXAMPLE = "{% include file.ext param='value' param2='value' %}" INCLUDES_DIR = '_includes' @@ -19,7 +19,7 @@ module Jekyll params = {} markup = @params - while match = MATCHER.match(markup) do + while match = VALID_SYNTAX.match(markup) do markup = markup[match.end(0)..-1] value = if match[2] @@ -50,15 +50,15 @@ Invalid syntax for include tag. File contains invalid characters or sequences: Valid syntax: - #{VALID_SYNTAX} + #{SYNTAX_EXAMPLE} eos end end def validate_params - full_matcher = Regexp.compile('\A\s*(?:' + MATCHER.to_s + '(?=\s|\z)\s*)*\z') - unless @params =~ full_matcher + full_VALID_SYNTAX = Regexp.compile('\A\s*(?:' + VALID_SYNTAX.to_s + '(?=\s|\z)\s*)*\z') + unless @params =~ full_VALID_SYNTAX raise SyntaxError.new <<-eos Invalid syntax for include tag: @@ -66,7 +66,7 @@ Invalid syntax for include tag: Valid syntax: - #{VALID_SYNTAX} + #{SYNTAX_EXAMPLE} eos end From 3e2ab58d56463fb0b93b996a1d5dc67c5f255199 Mon Sep 17 00:00:00 2001 From: Anatol Broder Date: Tue, 17 Sep 2013 20:34:04 +0200 Subject: [PATCH 06/13] Fix unruby paradigm --- lib/jekyll/tags/include.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index 89aad0ea..636b2436 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -92,7 +92,7 @@ eos end def validate_dir(dir, safe) - if File.symlink?(dir) && safe? + if File.symlink?(dir) && safe return "Includes directory '#{dir}' cannot be a symlink" end end @@ -100,7 +100,7 @@ eos def validate_file(file, safe) if !File.exists?(file) return "Included file '#{@file}' not found in '#{INCLUDES_DIR}' directory" - elsif File.symlink?(file) && safe? + elsif File.symlink?(file) && safe return "The included file '#{INCLUDES_DIR}/#{@file}' should not be a symlink" end end From e5f99e2798f1e2a3f18e867e8ccf911e6a91eef0 Mon Sep 17 00:00:00 2001 From: Anatol Broder Date: Tue, 17 Sep 2013 20:35:18 +0200 Subject: [PATCH 07/13] Remove return --- lib/jekyll/tags/include.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index 636b2436..8347aa2d 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -93,15 +93,15 @@ eos def validate_dir(dir, safe) if File.symlink?(dir) && safe - return "Includes directory '#{dir}' cannot be a symlink" + "Includes directory '#{dir}' cannot be a symlink" end end def validate_file(file, safe) if !File.exists?(file) - return "Included file '#{@file}' not found in '#{INCLUDES_DIR}' directory" + "Included file '#{@file}' not found in '#{INCLUDES_DIR}' directory" elsif File.symlink?(file) && safe - return "The included file '#{INCLUDES_DIR}/#{@file}' should not be a symlink" + "The included file '#{INCLUDES_DIR}/#{@file}' should not be a symlink" end end From 7f62e698376da64c91719317891edaa6d7e3b55d Mon Sep 17 00:00:00 2001 From: Anatol Broder Date: Tue, 17 Sep 2013 20:36:33 +0200 Subject: [PATCH 08/13] Remove obvious comment --- lib/jekyll/tags/include.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index 8347aa2d..c062f0e2 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -110,7 +110,6 @@ eos end # This method allows to modify the file content by inheriting from the class. - # Don’t refactor it. def source(file) File.read(file) end From 8017548bd0b3c33d555f667c94a6832ca6c82b8f Mon Sep 17 00:00:00 2001 From: Anatol Broder Date: Tue, 17 Sep 2013 20:46:01 +0200 Subject: [PATCH 09/13] Rename variable --- lib/jekyll/tags/include.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index c062f0e2..143555c8 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -57,8 +57,8 @@ eos end def validate_params - full_VALID_SYNTAX = Regexp.compile('\A\s*(?:' + VALID_SYNTAX.to_s + '(?=\s|\z)\s*)*\z') - unless @params =~ full_VALID_SYNTAX + full_valid_syntax = Regexp.compile('\A\s*(?:' + VALID_SYNTAX.to_s + '(?=\s|\z)\s*)*\z') + unless @params =~ full_valid_syntax raise SyntaxError.new <<-eos Invalid syntax for include tag: From 25519b38f69b0baae0cf7e0cca31a60c3b792b18 Mon Sep 17 00:00:00 2001 From: Anatol Broder Date: Tue, 17 Sep 2013 20:57:01 +0200 Subject: [PATCH 10/13] Validate file name as soon as possible --- lib/jekyll/tags/include.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index 143555c8..c4dea149 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -11,10 +11,11 @@ module Jekyll def initialize(tag_name, markup, tokens) super @file, @params = markup.strip.split(' ', 2); + validate_file_name end def parse_params(context) - validate_syntax + validate_params params = {} markup = @params @@ -35,12 +36,6 @@ module Jekyll params end - # ensure the entire markup string from start to end is valid syntax, and params are separated by spaces - def validate_syntax - validate_file_name - validate_params - end - def validate_file_name if @file !~ /^[a-zA-Z0-9_\/\.-]+$/ || @file =~ /\.\// || @file =~ /\/\./ raise SyntaxError.new <<-eos From 7cec996f90cd31ca2572e444717bfcc4ddee52fc Mon Sep 17 00:00:00 2001 From: Anatol Broder Date: Tue, 17 Sep 2013 21:41:11 +0200 Subject: [PATCH 11/13] Validate the entire markup as soon as possible --- lib/jekyll/tags/include.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index c4dea149..f02f6310 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -2,21 +2,24 @@ module Jekyll module Tags class IncludeTag < Liquid::Tag - VALID_SYNTAX = /([\w-]+)\s*=\s*(?:"([^"\\]*(?:\\.[^"\\]*)*)"|'([^'\\]*(?:\\.[^'\\]*)*)'|([\w\.-]+))/ - SYNTAX_EXAMPLE = "{% include file.ext param='value' param2='value' %}" + VALID_SYNTAX = /([\w-]+)\s*=\s*(?:"([^"\\]*(?:\\.[^"\\]*)*)"|'([^'\\]*(?:\\.[^'\\]*)*)'|([\w\.-]+))/ + INCLUDES_DIR = '_includes' def initialize(tag_name, markup, tokens) super @file, @params = markup.strip.split(' ', 2); + validate_syntax + end + + def validate_syntax validate_file_name + validate_params if @params end def parse_params(context) - validate_params - params = {} markup = @params From ec85c49de38641df094ddbef1e7a4749e63864a6 Mon Sep 17 00:00:00 2001 From: Anatol Broder Date: Tue, 17 Sep 2013 22:11:56 +0200 Subject: [PATCH 12/13] Change exception type --- lib/jekyll/tags/include.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index f02f6310..2b0d82b4 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -41,7 +41,7 @@ module Jekyll def validate_file_name if @file !~ /^[a-zA-Z0-9_\/\.-]+$/ || @file =~ /\.\// || @file =~ /\/\./ - raise SyntaxError.new <<-eos + raise ArgumentError.new <<-eos Invalid syntax for include tag. File contains invalid characters or sequences: #{@file} @@ -57,7 +57,7 @@ eos def validate_params full_valid_syntax = Regexp.compile('\A\s*(?:' + VALID_SYNTAX.to_s + '(?=\s|\z)\s*)*\z') unless @params =~ full_valid_syntax - raise SyntaxError.new <<-eos + raise ArgumentError.new <<-eos Invalid syntax for include tag: #{@params} From f97eed544ca64255999859e97c1dbe7d5c70192f Mon Sep 17 00:00:00 2001 From: Anatol Broder Date: Tue, 17 Sep 2013 22:20:41 +0200 Subject: [PATCH 13/13] Change exception type in tests --- test/test_tags.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test_tags.rb b/test/test_tags.rb index 5e25c1db..c58c9c7e 100644 --- a/test/test_tags.rb +++ b/test/test_tags.rb @@ -373,7 +373,7 @@ CONTENT end context "with invalid parameter syntax" do - should "throw a SyntaxError" do + should "throw a ArgumentError" do content = < 'pretty', 'source' => source_dir, 'destination' => dest_dir, 'read_posts' => true}) end @@ -392,7 +392,7 @@ title: Invalid parameter syntax {% include params.html params="value %} CONTENT - assert_raise SyntaxError, 'Did not raise exception on invalid "include" syntax' do + assert_raise ArgumentError, 'Did not raise exception on invalid "include" syntax' do create_post(content, {'permalink' => 'pretty', 'source' => source_dir, 'destination' => dest_dir, 'read_posts' => true}) end end