From be1bb60110fa6359a792986e46549ed15c15c2b0 Mon Sep 17 00:00:00 2001
From: kylepritchard <Kylepritchard@hotmail.co.uk>
Date: Thu, 8 Aug 2024 20:41:09 +0100
Subject: [PATCH] parser: update `@include` in templates, to work with relative
 paths & prevent recursive calls (#21943)

---
 vlib/v/TEMPLATES.md                           |   3 +-
 vlib/v/parser/tests/tmpl/recursive_fail.html  |   2 +
 vlib/v/parser/tests/tmpl/recursive_fail2.html |   2 +
 vlib/v/parser/tests/tmpl_recursion.out        |   5 +
 vlib/v/parser/tests/tmpl_recursion.vv         |   3 +
 vlib/v/parser/tmpl.v                          | 213 +++++++++++++-----
 vlib/v/parser/tmpl_test.v                     |  12 -
 vlib/v/tests/tmpl/base.html                   |   1 +
 .../templates => tests/tmpl}/index.html       |   2 +-
 vlib/v/tests/tmpl/nested/child.html           |   2 +
 .../tmpl/nested/nested_deeper/grandchild.html |   3 +
 vlib/v/tests/tmpl/parent.html                 |   4 +
 vlib/v/tests/tmpl_test.v                      |  42 ++++
 13 files changed, 223 insertions(+), 71 deletions(-)
 create mode 100644 vlib/v/parser/tests/tmpl/recursive_fail.html
 create mode 100644 vlib/v/parser/tests/tmpl/recursive_fail2.html
 create mode 100644 vlib/v/parser/tests/tmpl_recursion.out
 create mode 100644 vlib/v/parser/tests/tmpl_recursion.vv
 delete mode 100644 vlib/v/parser/tmpl_test.v
 create mode 100644 vlib/v/tests/tmpl/base.html
 rename vlib/v/{parser/templates => tests/tmpl}/index.html (98%)
 create mode 100644 vlib/v/tests/tmpl/nested/child.html
 create mode 100644 vlib/v/tests/tmpl/nested/nested_deeper/grandchild.html
 create mode 100644 vlib/v/tests/tmpl/parent.html

diff --git a/vlib/v/TEMPLATES.md b/vlib/v/TEMPLATES.md
index 28d0b9bac46382..791a545103d763 100644
--- a/vlib/v/TEMPLATES.md
+++ b/vlib/v/TEMPLATES.md
@@ -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:
 
@@ -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,
diff --git a/vlib/v/parser/tests/tmpl/recursive_fail.html b/vlib/v/parser/tests/tmpl/recursive_fail.html
new file mode 100644
index 00000000000000..78f3cb28258e36
--- /dev/null
+++ b/vlib/v/parser/tests/tmpl/recursive_fail.html
@@ -0,0 +1,2 @@
+<p>This makes a recursive call</p>
+@include 'recursive_fail2'
\ No newline at end of file
diff --git a/vlib/v/parser/tests/tmpl/recursive_fail2.html b/vlib/v/parser/tests/tmpl/recursive_fail2.html
new file mode 100644
index 00000000000000..f753e5b2127a02
--- /dev/null
+++ b/vlib/v/parser/tests/tmpl/recursive_fail2.html
@@ -0,0 +1,2 @@
+<p>This makes a recursive call</p>
+@include 'recursive_fail'
\ No newline at end of file
diff --git a/vlib/v/parser/tests/tmpl_recursion.out b/vlib/v/parser/tests/tmpl_recursion.out
new file mode 100644
index 00000000000000..b48765a11e5a6a
--- /dev/null
+++ b/vlib/v/parser/tests/tmpl_recursion.out
@@ -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'
+      | ~~~~~~~~~
+
diff --git a/vlib/v/parser/tests/tmpl_recursion.vv b/vlib/v/parser/tests/tmpl_recursion.vv
new file mode 100644
index 00000000000000..1425b464ff2d13
--- /dev/null
+++ b/vlib/v/parser/tests/tmpl_recursion.vv
@@ -0,0 +1,3 @@
+fn test_tmpl_catch_recursion() {
+	recurse := $tmpl('tmpl/recursive_fail.html')
+}
\ No newline at end of file
diff --git a/vlib/v/parser/tmpl.v b/vlib/v/parser/tmpl.v
index 9dc5066ac830f3..817e8fd235fa50 100644
--- a/vlib/v/parser/tmpl.v
+++ b/vlib/v/parser/tmpl.v
@@ -90,6 +90,128 @@ 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 {
@@ -97,7 +219,8 @@ pub fn (mut p Parser) compile_template_file(template_file string, fn_name string
 		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)
@@ -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
 		}
diff --git a/vlib/v/parser/tmpl_test.v b/vlib/v/parser/tmpl_test.v
deleted file mode 100644
index 1cc893bff9832d..00000000000000
--- a/vlib/v/parser/tmpl_test.v
+++ /dev/null
@@ -1,12 +0,0 @@
-// Add more examples of potentially buggy patterns in vlib/v/parser/templates/index.html
-fn test_tmpl_comptime() {
-	index := $tmpl('templates/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')
-}
diff --git a/vlib/v/tests/tmpl/base.html b/vlib/v/tests/tmpl/base.html
new file mode 100644
index 00000000000000..69fbe2b88e307e
--- /dev/null
+++ b/vlib/v/tests/tmpl/base.html
@@ -0,0 +1 @@
+<p>This is the base file</p>
\ No newline at end of file
diff --git a/vlib/v/parser/templates/index.html b/vlib/v/tests/tmpl/index.html
similarity index 98%
rename from vlib/v/parser/templates/index.html
rename to vlib/v/tests/tmpl/index.html
index c20d6a5638bd97..e2496f6fe00c1e 100644
--- a/vlib/v/parser/templates/index.html
+++ b/vlib/v/tests/tmpl/index.html
@@ -8,5 +8,5 @@
 	<br>Line ending with slash /
 	<br>Line ending with dollar $
 	<br>Line ending with caret ^
-</p>	
+</p>
 Last line.
diff --git a/vlib/v/tests/tmpl/nested/child.html b/vlib/v/tests/tmpl/nested/child.html
new file mode 100644
index 00000000000000..ad015e14341067
--- /dev/null
+++ b/vlib/v/tests/tmpl/nested/child.html
@@ -0,0 +1,2 @@
+@include '../base'
+<p>This is the child file</p>
diff --git a/vlib/v/tests/tmpl/nested/nested_deeper/grandchild.html b/vlib/v/tests/tmpl/nested/nested_deeper/grandchild.html
new file mode 100644
index 00000000000000..117e2d585921c2
--- /dev/null
+++ b/vlib/v/tests/tmpl/nested/nested_deeper/grandchild.html
@@ -0,0 +1,3 @@
+@include '../../base'
+@include '../child'
+<p>This is the grandchild file</p>
diff --git a/vlib/v/tests/tmpl/parent.html b/vlib/v/tests/tmpl/parent.html
new file mode 100644
index 00000000000000..1a52eac2be1008
--- /dev/null
+++ b/vlib/v/tests/tmpl/parent.html
@@ -0,0 +1,4 @@
+@include 'base'
+@include 'nested/child'
+@include 'nested/nested_deeper/grandchild'
+<p>This is the parent file</p>
\ No newline at end of file
diff --git a/vlib/v/tests/tmpl_test.v b/vlib/v/tests/tmpl_test.v
index 9b3ff34223625a..a80f6cdecd4b9b 100644
--- a/vlib/v/tests/tmpl_test.v
+++ b/vlib/v/tests/tmpl_test.v
@@ -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)
+}