Skip to content

Commit

Permalink
parser: update @include in templates, to work with relative paths &…
Browse files Browse the repository at this point in the history
… prevent recursive calls (#21943)
  • Loading branch information
kylepritchard authored Aug 8, 2024
1 parent eeea670 commit be1bb60
Show file tree
Hide file tree
Showing 13 changed files with 223 additions and 71 deletions.
3 changes: 2 additions & 1 deletion vlib/v/TEMPLATES.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ You can also write (and all other for condition syntaxes that are allowed in V):

The include directive is for including other html files (which will be processed as well)
and consists of two parts, the `@include` tag and a following `'<path>'` string.
The path parameter is relative to the `/templates` directory in the corresponding project.
The path parameter is relative to the template file being called.

### Example for the folder structure of a project using templates:

Expand All @@ -138,6 +138,7 @@ Project root
> Note that there shouldn't be a file suffix,
> it is automatically appended and only allows `html` files.

## js

The js directive consists of two parts, the `@js` tag and `'<path>'` string,
Expand Down
2 changes: 2 additions & 0 deletions vlib/v/parser/tests/tmpl/recursive_fail.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<p>This makes a recursive call</p>
@include 'recursive_fail2'
2 changes: 2 additions & 0 deletions vlib/v/parser/tests/tmpl/recursive_fail2.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<p>This makes a recursive call</p>
@include 'recursive_fail'
5 changes: 5 additions & 0 deletions vlib/v/parser/tests/tmpl_recursion.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
vlib/v/parser/tests/tmpl/recursive_fail2.html:2:1: error: A recursive call is being made on template recursive_fail
1 | <p>This makes a recursive call</p>
2 | @include 'recursive_fail'
| ~~~~~~~~~

3 changes: 3 additions & 0 deletions vlib/v/parser/tests/tmpl_recursion.vv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn test_tmpl_catch_recursion() {
recurse := $tmpl('tmpl/recursive_fail.html')
}
213 changes: 156 additions & 57 deletions vlib/v/parser/tmpl.v
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,137 @@ fn insert_template_code(fn_name string, tmpl_str_start string, line string) stri
return rline
}

// struct to track dependecies and cache templates for reuse without io
struct DependencyCache {
pub mut:
dependencies map[string][]string
cache map[string][]string
}

// custom error to handle issues when including template files
struct IncludeError {
Error
pub:
calling_file string
line_nr int
position int
col int
message string
}

fn (err IncludeError) msg() string {
return err.message
}

fn (err IncludeError) line_nr() int {
return err.line_nr
}

fn (err IncludeError) pos() int {
return err.position
}

fn (err IncludeError) calling_file() string {
return err.calling_file
}

fn (err IncludeError) col() int {
return err.col
}

fn (mut p Parser) process_includes(calling_file string, line_number int, line string, mut dc DependencyCache) ![]string {
base_path := os.dir(calling_file)
mut tline_number := line_number
mut file_name := if line.contains('"') {
line.split('"')[1]
} else if line.contains("'") {
line.split("'")[1]
} else {
position := line.index('@include ') or { 0 }
return &IncludeError{
calling_file: calling_file
line_nr: tline_number // line_number
position: position + '@include '.len
col: position + '@include '.len
message: 'path for @include must be quoted with \' or "'
}
}
mut file_ext := os.file_ext(file_name)
if file_ext == '' {
file_ext = '.html'
}
file_name = file_name.replace(file_ext, '')
mut file_path := os.real_path(os.join_path_single(base_path, '${file_name}${file_ext}'))

if !os.exists(file_path) && !file_name.contains('../') {
// the calling file is probably original way (relative to calling file) and works from the root folder
path_arr := base_path.split_any('/\\')
idx := path_arr.index('templates')
root_path := path_arr[..idx + 1].join('/')
file_name = file_name.rsplit('../')[0]
file_path = os.real_path(os.join_path_single(root_path, '${file_name}${file_ext}'))
}

// If file hasnt been called before then add to dependency tree
if !dc.dependencies[file_path].contains(calling_file) {
dc.dependencies[file_path] << calling_file
}

// Circular import detection
for callee in dc.dependencies[file_path] {
if dc.dependencies[callee].contains(file_path) {
return &IncludeError{
calling_file: calling_file
line_nr: tline_number
position: line.index('@include ') or { 0 }
message: 'A recursive call is being made on template ${file_name}'
}
}
}
mut file_content := []string{}
if file_path in dc.cache {
file_content = dc.cache[file_path]
} else {
file_content = os.read_lines(file_path) or {
position := line.index('@include ') or { 0 } + '@include '.len
return &IncludeError{
calling_file: calling_file
line_nr: tline_number // line_number
position: position
message: 'Reading file `${file_name}` from path: ${file_path} failed'
}
}
}
// no errors detected in calling file - reset tline_number (error reporting)
tline_number = 1

// loop over the imported file
for i, l in file_content {
if l.contains('@include ') {
processed := p.process_includes(file_path, tline_number, l, mut dc) or { return err }
file_content.delete(i) // remove the include line
for processed_line in processed.reverse() {
file_content.insert(i, processed_line)
tline_number--
}
}
}
// Add template to parser for reloading
p.template_paths << file_path
// Add the imported template to the cache
dc.cache[file_path] = file_content
return file_content
}

// compile_file compiles the content of a file by the given path as a template
pub fn (mut p Parser) compile_template_file(template_file string, fn_name string) string {
mut lines := os.read_lines(template_file) or {
p.error('reading from ${template_file} failed')
return ''
}
p.template_paths << template_file
basepath := os.dir(template_file)
// create a new Dependency tree & cache any templates to avoid further io
mut dc := DependencyCache{}
lstartlength := lines.len * 30
tmpl_str_start := "\tsb_${fn_name}.write_string('"
mut source := strings.new_builder(1000)
Expand Down Expand Up @@ -163,65 +286,41 @@ fn vweb_tmpl_${fn_name}() string {
}
if line.contains('@include ') {
lines.delete(i)
// Allow single or double quoted paths.
mut file_name := if line.contains('"') {
line.split('"')[1]
} else if line.contains("'") {
line.split("'")[1]
} else {
s := '@include '
position := line.index(s) or { 0 }
p.error_with_error(errors.Error{
message: 'path for @include must be quoted with \' or "'
file_path: template_file
pos: token.Pos{
len: s.len
line_nr: tline_number
pos: start_of_line_pos + position + s.len
col: position + s.len
last_line: lines.len + 1
}
reporter: .parser
})
''
}
mut file_ext := os.file_ext(file_name)
if file_ext == '' {
file_ext = '.html'
}
file_name = file_name.replace(file_ext, '')
// relative path, starting with the current folder
mut templates_folder := os.real_path(basepath)
if file_name.contains('/') && file_name.starts_with('/') {
// an absolute path
templates_folder = ''
}
file_path := os.real_path(os.join_path_single(templates_folder, '${file_name}${file_ext}'))
$if trace_tmpl ? {
eprintln('>>> basepath: "${basepath}" , template_file: "${template_file}" , fn_name: "${fn_name}" , @include line: "${line}" , file_name: "${file_name}" , file_ext: "${file_ext}" , templates_folder: "${templates_folder}" , file_path: "${file_path}"')
}
file_content := os.read_file(file_path) or {
position := line.index('@include ') or { 0 } + '@include '.len
p.error_with_error(errors.Error{
message: 'Reading file ${file_name} from path: ${file_path} failed'
details: "Failed to @include '${file_name}'"
file_path: template_file
pos: token.Pos{
len: '@include '.len + file_name.len
line_nr: tline_number
pos: start_of_line_pos + position
last_line: lines.len
}
reporter: .parser
})
''
resolved := p.process_includes(template_file, tline_number, line, mut &dc) or {
if err is IncludeError {
p.error_with_error(errors.Error{
message: err.msg()
file_path: err.calling_file()
pos: token.Pos{
len: '@include '.len
line_nr: err.line_nr()
pos: start_of_line_pos + err.pos()
col: err.col()
last_line: lines.len
}
reporter: .parser
})
[]string{}
} else {
p.error_with_error(errors.Error{
message: 'An unknown error has occurred'
file_path: template_file
pos: token.Pos{
len: '@include '.len
line_nr: tline_number
pos: start_of_line_pos
last_line: lines.len
}
reporter: .parser
})
[]string{}
}
}
p.template_paths << file_path
file_splitted := file_content.split_into_lines().reverse()
for f in file_splitted {
for resolved_line in resolved.reverse() {
tline_number--
lines.insert(i, f)
lines.insert(i, resolved_line)
}

i--
continue
}
Expand Down
12 changes: 0 additions & 12 deletions vlib/v/parser/tmpl_test.v

This file was deleted.

1 change: 1 addition & 0 deletions vlib/v/tests/tmpl/base.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p>This is the base file</p>
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@
<br>Line ending with slash /
<br>Line ending with dollar $
<br>Line ending with caret ^
</p>
</p>
Last line.
2 changes: 2 additions & 0 deletions vlib/v/tests/tmpl/nested/child.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@include '../base'
<p>This is the child file</p>
3 changes: 3 additions & 0 deletions vlib/v/tests/tmpl/nested/nested_deeper/grandchild.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
@include '../../base'
@include '../child'
<p>This is the grandchild file</p>
4 changes: 4 additions & 0 deletions vlib/v/tests/tmpl/parent.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
@include 'base'
@include 'nested/child'
@include 'nested/nested_deeper/grandchild'
<p>This is the parent file</p>
42 changes: 42 additions & 0 deletions vlib/v/tests/tmpl_test.v
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,45 @@ fn test_tmpl_interpolation() {
fn my_fn(s string) string {
return s
}

// Add more examples of potentially buggy patterns in vlib/v/tests/tmpl/index.html
fn test_tmpl_comptime() {
index := $tmpl('tmpl/index.html').trim_space()
// dump(index)
assert index.contains('<br>Line ending with percent %\n')
assert index.contains('<br>Line ending with at $\n')
assert index.contains('<br>Line ending with ampersand &\n')
assert index.contains('<br>Line ending with hash #\n')
assert index.contains('<br>Line ending with slash /\n')
assert index.contains('<br>Line ending with dollar $\n')
assert index.contains('<br>Line ending with caret ^\n')
}

// Add a tests for @include

// File contents for building repsonse
const base = '<p>This is the base file</p>'
const child = '<p>This is the child file</p>'
const grandchild = '<p>This is the grandchild file</p>'
const parent = '<p>This is the parent file</p>'

// Call the parent file which contains all child templates
fn test_tmpl_include_parent() {
expected := [base, base, child, base, base, child, grandchild, parent].join('\n')
parent_tmpl := $tmpl('tmpl/parent.html')
assert parent_tmpl.contains(expected)
}

// Test the child template which calls parent template
fn test_tmpl_include_child() {
expected := [base, child].join('\n')
child_tmpl := $tmpl('tmpl/nested/child.html')
assert child_tmpl.contains(expected)
}

// Test the grandchild templates calling both parent and grandparent templates
fn test_tmpl_include_grandchild() {
expected := [base, base, child, grandchild].join('\n')
child_tmpl := $tmpl('tmpl/nested/nested_deeper/grandchild.html')
assert child_tmpl.contains(expected)
}

0 comments on commit be1bb60

Please sign in to comment.