Fix document save race in parser_aux::load_and_save and creator::crea…

…te_document (#151)
J-F-Liu · Nov 22, 2021 · 53090ad · 53090ad
1 parent fec4729
commit 53090ad
Show file tree

Hide file tree

Showing 3 changed files with 136 additions and 55 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -26,6 +26,7 @@ log = "^0.4"
 rayon = { version = "^1.4", optional = true }
 nom = { version = "^6.0", optional = true }
 weezl = "0.1.4"
+lazy_static = "^1.4"
 
 [features]
 default = ["chrono_time", "pom_parser"]

diff --git a/src/creator.rs b/src/creator.rs
@@ -113,58 +113,129 @@ impl Document {
     }
 }
 
-#[test]
-fn create_document() {
-    use super::content::*;
-    use super::Stream;
-
-    let mut doc = Document::with_version("1.5");
-    let info_id = doc.add_object(dictionary! {
-        "Title" => Object::string_literal("Create PDF document example"),
-        "Creator" => Object::string_literal("https://crates.io/crates/lopdf"),
-        "CreationDate" => time::OffsetDateTime::now_utc(),
-    });
-    let pages_id = doc.new_object_id();
-    let font_id = doc.add_object(dictionary! {
-        "Type" => "Font",
-        "Subtype" => "Type1",
-        "BaseFont" => "Courier",
-    });
-    let resources_id = doc.add_object(dictionary! {
-        "Font" => dictionary! {
-            "F1" => font_id,
-        },
-    });
-    let content = Content {
-        operations: vec![
-            Operation::new("BT", vec![]),
-            Operation::new("Tf", vec!["F1".into(), 48.into()]),
-            Operation::new("Td", vec![100.into(), 600.into()]),
-            Operation::new("Tj", vec![Object::string_literal("Hello World!")]),
-            Operation::new("ET", vec![]),
-        ],
-    };
-    let content_id = doc.add_object(Stream::new(dictionary! {}, content.encode().unwrap()));
-    let page_id = doc.add_object(dictionary! {
-        "Type" => "Page",
-        "Parent" => pages_id,
-        "Contents" => content_id,
-    });
-    let pages = dictionary! {
-        "Type" => "Pages",
-        "Kids" => vec![page_id.into()],
-        "Count" => 1,
-        "Resources" => resources_id,
-        "MediaBox" => vec![0.into(), 0.into(), 595.into(), 842.into()],
-    };
-    doc.objects.insert(pages_id, Object::Dictionary(pages));
-    let catalog_id = doc.add_object(dictionary! {
-        "Type" => "Catalog",
-        "Pages" => pages_id,
-    });
-    doc.trailer.set("Root", catalog_id);
-    doc.trailer.set("Info", info_id);
-    doc.compress();
-
-    doc.save("test_1_create.pdf").unwrap();
+#[cfg(test)]
+pub mod tests {
+    use std::fs::remove_file;
+    use std::path::Path;
+    use std::sync::Mutex;
+
+    use crate::content::*;
+    use crate::{Document, Object, Stream};
+    use lazy_static::lazy_static;
+
+    lazy_static! {
+        /// Tests that save and share files are vulnerable to race conditions
+        /// Use a mutex so only one test at a time has access to update it
+        /// The lock will last for the length of the let block in the tests
+        static ref DOC_FILE_MUTEX: Mutex<()> = Mutex::new(());
+    }
+
+    /// Create and return a document for testing
+    pub fn create_document() -> Document {
+        let mut doc = Document::with_version("1.5");
+        let info_id = doc.add_object(dictionary! {
+            "Title" => Object::string_literal("Create PDF document example"),
+            "Creator" => Object::string_literal("https://crates.io/crates/lopdf"),
+            "CreationDate" => time::OffsetDateTime::now_utc(),
+        });
+        let pages_id = doc.new_object_id();
+        let font_id = doc.add_object(dictionary! {
+            "Type" => "Font",
+            "Subtype" => "Type1",
+            "BaseFont" => "Courier",
+        });
+        let resources_id = doc.add_object(dictionary! {
+            "Font" => dictionary! {
+                "F1" => font_id,
+            },
+        });
+        let content = Content {
+            operations: vec![
+                Operation::new("BT", vec![]),
+                Operation::new("Tf", vec!["F1".into(), 48.into()]),
+                Operation::new("Td", vec![100.into(), 600.into()]),
+                Operation::new("Tj", vec![Object::string_literal("Hello World!")]),
+                Operation::new("ET", vec![]),
+            ],
+        };
+        let content_id = doc.add_object(Stream::new(dictionary! {}, content.encode().unwrap()));
+        let page_id = doc.add_object(dictionary! {
+            "Type" => "Page",
+            "Parent" => pages_id,
+            "Contents" => content_id,
+        });
+        let pages = dictionary! {
+            "Type" => "Pages",
+            "Kids" => vec![page_id.into()],
+            "Count" => 1,
+            "Resources" => resources_id,
+            "MediaBox" => vec![0.into(), 0.into(), 595.into(), 842.into()],
+        };
+        doc.objects.insert(pages_id, Object::Dictionary(pages));
+        let catalog_id = doc.add_object(dictionary! {
+            "Type" => "Catalog",
+            "Pages" => pages_id,
+        });
+        doc.trailer.set("Root", catalog_id);
+        doc.trailer.set("Info", info_id);
+        doc.compress();
+
+        doc
+    }
+
+    /// Save a document
+    pub fn save_document(filename: &String, doc: &mut Document) {
+        let res = doc.save(filename);
+
+        assert!(match res {
+            Ok(_file) => true,
+            Err(_e) => false,
+        });
+    }
+
+    /// Remove a document
+    pub fn remove_document(filename: &String) {
+        let path = Path::new(&filename);
+
+        assert_ne!(path.as_os_str(), "");
+        assert_ne!(path.as_os_str(), "/");
+
+        let exists = path.exists();
+        assert!(exists);
+
+        if exists {
+            remove_file(path).unwrap();
+        }
+    }
+
+    #[test]
+    fn create_document_creates_document() {
+        let filename = String::from("test_1_create.pdf");
+        let path = Path::new(&filename);
+        let mut doc = create_document();
+
+        {
+            let _m = DOC_FILE_MUTEX.lock().unwrap();
+            save_document(&filename, &mut doc);
+            assert!(path.exists());
+            remove_document(&filename);
+        }
+    }
+
+    #[test]
+    fn remove_document_removes_document() {
+        let filename = String::from("test_1_create.pdf");
+
+        let path = Path::new(&filename);
+
+        let mut doc = create_document();
+
+        {
+            let _m = DOC_FILE_MUTEX.lock().unwrap();
+            save_document(&filename, &mut doc);
+            assert!(path.exists());
+            remove_document(&filename);
+            assert!(!path.exists());
+        }
+    }
 }
diff --git a/src/parser_aux.rs b/src/parser_aux.rs
@@ -270,14 +270,23 @@ fn parse_integer_array(array: &Object) -> Result<Vec<i64>> {
 #[test]
 fn load_and_save() {
     // test load_from() and save_to()
+    use crate::creator::tests::{create_document, remove_document, save_document};
     use std::fs::File;
     use std::io::Cursor;
 
-    let in_file = File::open("test_1_create.pdf").unwrap();
+    let filename = String::from("test_1_load_and_save.pdf");
+    let mut doc = create_document();
+
+    save_document(&filename, &mut doc);
+
+    let in_file = File::open("test_1_load_and_save.pdf").unwrap();
     let mut in_doc = Document::load_from(in_file).unwrap();
 
     let out_buf = Vec::new();
     let mut memory_cursor = Cursor::new(out_buf);
     in_doc.save_to(&mut memory_cursor).unwrap();
     assert!(!memory_cursor.get_ref().is_empty());
+
+    // Clean up the saved document
+    remove_document(&filename);
 }