From 41b88fa6456dad25c393e781a8ce4eb7e97dd6c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?M=C3=B4she=20van=20der=20Sterre?= <me@moshe.nl>
Date: Sat, 8 Feb 2025 19:50:00 +0100
Subject: [PATCH] Modify the self-referencing Parsed struct to use the
 ouroboros crate.

Removing this usage of unsafe rust is aimed at lowering the maintenance
burden and risk of potential future memory-safety bugs. Future changes
to this part of the code won't need to be (re)analyzed manually to
confirm that the safety guarantees continue to hold, and it will be
easier for third-party organisations to be confident that askama is
memory-safe.
---
 askama_parser/Cargo.toml |  1 +
 askama_parser/src/lib.rs | 48 ++++++++++++++++++++++++++--------------
 2 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/askama_parser/Cargo.toml b/askama_parser/Cargo.toml
index 92727a91..e7fa6fb9 100644
--- a/askama_parser/Cargo.toml
+++ b/askama_parser/Cargo.toml
@@ -15,6 +15,7 @@ rust-version = "1.65"
 
 [dependencies]
 nom = { version = "7", default-features = false, features = ["alloc"] }
+ouroboros = "0.18.5"
 
 [dev-dependencies]
 criterion = "0.5"
diff --git a/askama_parser/src/lib.rs b/askama_parser/src/lib.rs
index 09218aa2..abd9ba58 100644
--- a/askama_parser/src/lib.rs
+++ b/askama_parser/src/lib.rs
@@ -25,19 +25,36 @@ pub use node::Node;
 mod tests;
 
 mod _parsed {
+    use std::fmt;
     use std::path::Path;
     use std::rc::Rc;
-    use std::{fmt, mem};
+
+    use ouroboros::self_referencing;
 
     use super::node::Node;
     use super::{Ast, ParseError, Syntax};
 
-    #[derive(Default)]
-    pub struct Parsed {
-        // `source` must outlive `ast`, so `ast` must be declared before `source`
-        ast: Ast<'static>,
-        #[allow(dead_code)]
+    #[self_referencing]
+    struct ParsedInternal {
         source: String,
+        #[covariant]
+        #[borrows(source)]
+        ast: Ast<'this>,
+    }
+
+    pub struct Parsed {
+        internal: ParsedInternal,
+    }
+
+    impl Default for Parsed {
+        fn default() -> Self {
+            let internal = ParsedInternalBuilder {
+                source: Default::default(),
+                ast_builder: |_| Default::default(),
+            }
+            .build();
+            Parsed { internal }
+        }
     }
 
     impl Parsed {
@@ -48,31 +65,30 @@ mod _parsed {
             file_path: Option<Rc<Path>>,
             syntax: &Syntax<'_>,
         ) -> Result<Self, ParseError> {
-            // Self-referential borrowing: `self` will keep the source alive as `String`,
-            // internally we will transmute it to `&'static str` to satisfy the compiler.
-            // However, we only expose the nodes with a lifetime limited to `self`.
-            let src = unsafe { mem::transmute::<&str, &'static str>(source.as_str()) };
-            let ast = Ast::from_str(src, file_path, syntax)?;
-            Ok(Self { ast, source })
+            let internal = ParsedInternalTryBuilder {
+                source,
+                ast_builder: |src: &String| Ast::from_str(src, file_path, syntax),
+            }
+            .try_build()?;
+            Ok(Self { internal })
         }
 
-        // The return value's lifetime must be limited to `self` to uphold the unsafe invariant.
         pub fn nodes(&self) -> &[Node<'_>] {
-            &self.ast.nodes
+            &self.internal.borrow_ast().nodes
         }
     }
 
     impl fmt::Debug for Parsed {
         fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
             f.debug_struct("Parsed")
-                .field("nodes", &self.ast.nodes)
+                .field("nodes", &self.internal.borrow_ast().nodes)
                 .finish_non_exhaustive()
         }
     }
 
     impl PartialEq for Parsed {
         fn eq(&self, other: &Self) -> bool {
-            self.ast.nodes == other.ast.nodes
+            self.internal.borrow_ast().nodes == other.internal.borrow_ast().nodes
         }
     }
 }