From ed5c755bb69991727b284fc3c255d77e0d2fe1da Mon Sep 17 00:00:00 2001
From: Sergio Benitez <sb@sergio.bz>
Date: Sat, 14 Oct 2023 15:00:36 -0700
Subject: [PATCH] Avoid using 'glob' to walk templates directory.

Previously, `dyn_templates` walked the user-provided `template_dir` path by
constructing a glob pattern prefixed with `template_dir`. If `template_dir`
contained characters recognized by the glob pattern parser, then at best the
pattern failed to parse, and at worst, incorrect directories were searched.

This commit removes the use of `glob` to walk the templates directory and
instead uses `walkdir`, obviating the issues described above.

Fixes #2627.
---
 contrib/dyn_templates/Cargo.toml              |  2 +-
 contrib/dyn_templates/src/context.rs          | 33 ++++++++++++-------
 contrib/dyn_templates/tests/templates.rs      | 10 ++++++
 .../templates/tera/[test]/html_test.html.tera |  5 +++
 4 files changed, 38 insertions(+), 12 deletions(-)
 create mode 100644 contrib/dyn_templates/tests/templates/tera/[test]/html_test.html.tera

diff --git a/contrib/dyn_templates/Cargo.toml b/contrib/dyn_templates/Cargo.toml
index 166bdf8fd6..d4386d2b2b 100644
--- a/contrib/dyn_templates/Cargo.toml
+++ b/contrib/dyn_templates/Cargo.toml
@@ -17,7 +17,7 @@ tera = ["tera_"]
 handlebars = ["handlebars_"]
 
 [dependencies]
-glob = "0.3"
+walkdir = "2.4"
 notify = "6"
 normpath = "1"
 
diff --git a/contrib/dyn_templates/src/context.rs b/contrib/dyn_templates/src/context.rs
index cf698823d9..ecdd93212a 100644
--- a/contrib/dyn_templates/src/context.rs
+++ b/contrib/dyn_templates/src/context.rs
@@ -26,6 +26,12 @@ impl Context {
     /// template engine, and store all of the initialized state in a `Context`
     /// structure, which is returned if all goes well.
     pub fn initialize(root: &Path, callback: &Callback) -> Option<Context> {
+        fn is_file_with_ext(entry: &walkdir::DirEntry, ext: &str) -> bool {
+            let is_file = entry.file_type().is_file();
+            let has_ext = entry.path().extension().map_or(false, |e| e == ext);
+            is_file && has_ext
+        }
+
         let root = match root.normalize() {
             Ok(root) => root.into_path_buf(),
             Err(e) => {
@@ -35,18 +41,23 @@ impl Context {
         };
 
         let mut templates: HashMap<String, TemplateInfo> = HashMap::new();
-        for ext in Engines::ENABLED_EXTENSIONS {
-            let mut glob_path = root.join("**").join("*");
-            glob_path.set_extension(ext);
-            let glob_path = glob_path.to_str().expect("valid glob path string");
+        for &ext in Engines::ENABLED_EXTENSIONS {
+            for entry in walkdir::WalkDir::new(&root).follow_links(true) {
+                let entry = match entry {
+                    Ok(entry) if is_file_with_ext(&entry, ext) => entry,
+                    Ok(_) | Err(_) => continue,
+                };
 
-            for path in glob::glob(glob_path).unwrap().filter_map(Result::ok) {
-                let (name, data_type_str) = split_path(&root, &path);
+                let (name, data_type_str) = split_path(&root, entry.path());
                 if let Some(info) = templates.get(&*name) {
-                    warn_!("Template name '{}' does not have a unique path.", name);
-                    info_!("Existing path: {:?}", info.path);
-                    info_!("Additional path: {:?}", path);
-                    warn_!("Using existing path for template '{}'.", name);
+                    warn_!("Template name '{}' does not have a unique source.", name);
+                    match info.path {
+                        Some(ref path) => info_!("Existing path: {:?}", path),
+                        None => info_!("Existing Content-Type: {}", info.data_type),
+                    }
+
+                    info_!("Additional path: {:?}", entry.path());
+                    warn_!("Keeping existing template '{}'.", name);
                     continue;
                 }
 
@@ -55,7 +66,7 @@ impl Context {
                     .unwrap_or(ContentType::Text);
 
                 templates.insert(name, TemplateInfo {
-                    path: Some(path.clone()),
+                    path: Some(entry.into_path()),
                     engine_ext: ext,
                     data_type,
                 });
diff --git a/contrib/dyn_templates/tests/templates.rs b/contrib/dyn_templates/tests/templates.rs
index c73927f94a..8db5923df9 100644
--- a/contrib/dyn_templates/tests/templates.rs
+++ b/contrib/dyn_templates/tests/templates.rs
@@ -237,6 +237,16 @@ mod tera_tests {
         assert_eq!(md_rendered, Some((ContentType::HTML, ESCAPED_EXPECTED.into())));
     }
 
+    #[async_test]
+    async fn test_globby_paths() {
+        use rocket::local::asynchronous::Client;
+
+        let client = Client::debug(rocket()).await.unwrap();
+        let req = client.get("/");
+        let metadata = Metadata::from_request(&req).await.unwrap();
+        assert!(metadata.contains_template("tera/[test]/html_test"));
+    }
+
     // u128 is not supported. enable when it is.
     // #[test]
     // fn test_tera_u128() {
diff --git a/contrib/dyn_templates/tests/templates/tera/[test]/html_test.html.tera b/contrib/dyn_templates/tests/templates/tera/[test]/html_test.html.tera
new file mode 100644
index 0000000000..17cd1b2023
--- /dev/null
+++ b/contrib/dyn_templates/tests/templates/tera/[test]/html_test.html.tera
@@ -0,0 +1,5 @@
+{% extends "tera/base" %}
+{% block title %}{{ title }}{% endblock title %}
+{% block content %}
+{{ content }}
+{% endblock content %}