Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Symlink Metadata and Double Packing Avoidance #256

Closed
wants to merge 5 commits into from
Closed

Symlink Metadata and Double Packing Avoidance #256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4a9dedf
Added capability to copy Nodes without modification
organizedgrime Apr 6, 2023
bdc2022
Added functions for inserting and retrieving PrivateFile based symlin…
organizedgrime Apr 18, 2023
30fc252
fix: fmt and clippy
organizedgrime Apr 25, 2023
ab70ee7
fix: compilation error for lack of import; re-privatized ratchet field
organizedgrime May 5, 2023
c9959bd
fix: Added missing documentation for new functions
organizedgrime May 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions wnfs-common/src/metadata.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,14 +235,7 @@ impl TryFrom<&str> for NodeType {

impl From<&NodeType> for String {
fn from(r#type: &NodeType) -> Self {
match r#type {
NodeType::PrivateDirectory => "wnfs/priv/dir".into(),
NodeType::PrivateFile => "wnfs/priv/file".into(),
NodeType::PublicDirectory => "wnfs/pub/dir".into(),
NodeType::PublicFile => "wnfs/pub/file".into(),
NodeType::TemporalSharePointer => "wnfs/share/temporal".into(),
NodeType::SnapshotSharePointer => "wnfs/share/snapshot".into(),
}
r#type.to_string()
}
}
Comment on lines 236 to 240
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch


Expand Down
199 changes: 198 additions & 1 deletion wnfs/src/private/directory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{error::FsError, traits::Id, SearchResult};
use anyhow::{bail, ensure, Result};
use async_once_cell::OnceCell;
use chrono::{DateTime, Utc};
use libipld::Cid;
use libipld::{Cid, Ipld};
use rand_core::RngCore;
use semver::Version;
use serde::{de::Error as DeError, ser::Error as SerError, Deserialize, Deserializer, Serialize};
Expand Down Expand Up @@ -915,6 +915,94 @@ impl PrivateDirectory {
Ok(())
}

/// Write a Symlink to the filesystem with the reference path at the path segments specified
///
/// # Examples
///
/// ```
/// use std::rc::Rc;
///
/// use chrono::Utc;
/// use rand::thread_rng;
/// use wnfs::{
/// private::{PrivateForest, PrivateRef, PrivateDirectory},
/// common::{BlockStore, MemoryBlockStore},
/// namefilter::Namefilter,
/// };
///
/// #[async_std::main]
/// async fn main() {
/// let store = &mut MemoryBlockStore::default();
/// let rng = &mut thread_rng();
/// let forest = &mut Rc::new(PrivateForest::new());
/// let root_dir = &mut Rc::new(PrivateDirectory::new(
/// Namefilter::default(),
/// Utc::now(),
/// rng,
/// ));
/// let sym_path = "/pictures/meows".to_string();
/// let path_segments = &["pictures".into(), "cats".into()];
///
/// root_dir
/// .write_symlink(sym_path.clone(), path_segments, true, Utc::now(), forest, store, rng)
/// .await
/// .unwrap();
///
/// let symlink = root_dir.get_node(path_segments, true, forest, store)
/// .await
/// .unwrap()
/// .expect("Symlink should be present")
/// .as_file()
/// .unwrap();
///
/// let path = symlink.symlink_origin();
/// assert!(path.is_some());
/// assert_eq!(path, Some(sym_path));
/// }
/// ```
#[allow(clippy::too_many_arguments)]
pub async fn write_symlink(
self: &mut Rc<Self>,
path: String,
path_segments: &[String],
search_latest: bool,
time: DateTime<Utc>,
forest: &PrivateForest,
store: &impl BlockStore,
rng: &mut impl RngCore,
) -> Result<()> {
let (path_segments, filename) = crate::utils::split_last(path_segments)?;

let dir = self
.get_or_create_leaf_dir_mut(path_segments, time, search_latest, forest, store, rng)
.await?;

match dir
.lookup_node_mut(filename, search_latest, forest, store)
.await?
{
Some(PrivateNode::File(file)) => {
let file = file.prepare_next_revision()?;
file.content.content = super::FileContent::Inline { data: vec![] };
file.content.metadata.upsert_mtime(time);
// Write the path into the Metadata HashMap
file.content
.metadata
.0
.insert(String::from("symlink"), Ipld::String(path));
}
Some(PrivateNode::Dir(_)) => bail!(FsError::DirectoryAlreadyExists),
None => {
let file =
PrivateFile::new_symlink(path, dir.header.bare_name.clone(), time, rng).await?;
let link = PrivateLink::with_file(file);
dir.content.entries.insert(filename.to_string(), link);
}
};

Ok(())
}

/// Returns names and metadata of directory's immediate children.
///
/// # Examples
Expand Down Expand Up @@ -1123,6 +1211,33 @@ impl PrivateDirectory {
Ok(())
}

/// Attaches a node to the specified directory without modifying the node.
#[allow(clippy::too_many_arguments)]
async fn attach_link(
self: &mut Rc<Self>,
node: PrivateNode,
path_segments: &[String],
search_latest: bool,
forest: &mut Rc<PrivateForest>,
store: &impl BlockStore,
) -> Result<()> {
let (path, node_name) = crate::utils::split_last(path_segments)?;
let SearchResult::Found(dir) = self.get_leaf_dir_mut(path, search_latest, forest, store).await? else {
bail!(FsError::NotFound);
};

ensure!(
!dir.content.entries.contains_key(node_name),
FsError::FileAlreadyExists
);

dir.content
.entries
.insert(node_name.clone(), PrivateLink::from(node));

Ok(())
}
Comment on lines +1216 to +1239
Copy link
Member

@appcypher appcypher May 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting use-case. So the in-memory tree structure might be good enough for fast copies but not updating the ancestry will leak secrets about unrelated directories. I'm trying to think of a way to handle this use-case. Will need to meet with @matheus23 for this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm...

I want to point out that this does something to the way that permissions work in WNFS:
In WNFS, each PrivateNode that's stored has a bunch of inumbers in its accumulator (the Namefilter in this version).
One can think of each inumber as a "path segment" in a path.
You can prove (to a third party) write access to a certain PrivateNode by providing a signature of an inumber, signed by the "root owner" of a WNFS.

If you simply copy over a PrivateRef from one place to another, you won't be updating the whole Namefilter structure (this is what the attach logic is doing!).
Concretely this means:
If you have a file at /Doc/Test.png and you symlink it to /Images/Test.png, then someone who has write access to /Images won't be able to modify Test.png! (Read access will work just fine)

This breaks an invariant in WNFS: That if you have write access to a directory, you'll have write access to its contents (recursively).

I guess in theory you could fix this by including both /Doc's and /Images' inumbers in the Test.png PrivateNode.

Can we go back to the drawing board on this? That'd probably be the "right" way of solving deduplication in rs-wnfs (at least on the file level). LMK if that makes sense to you @organizedgrime and let's brainstorm a bit about how this could look like API-wise (probably helpful if we do it live).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess in theory you could fix this by including both /Doc's and /Images' inumbers in the Test.png PrivateNode.

I thought about this again and now I think this doesn't work (with name accumulators).


/// Moves a file or directory from one path to another.
///
/// # Examples
Expand Down Expand Up @@ -1298,6 +1413,88 @@ impl PrivateDirectory {
.await
}

/// Copies a file or directory from one path to another without modifying it
///
/// # Examples
///
/// ```
/// use std::rc::Rc;
///
/// use chrono::Utc;
/// use rand::thread_rng;
///
/// use wnfs::{
/// private::{PrivateForest, PrivateRef, PrivateDirectory},
/// common::{BlockStore, MemoryBlockStore},
/// namefilter::Namefilter,
/// };
///
/// #[async_std::main]
/// async fn main() {
/// let store = &mut MemoryBlockStore::default();
/// let rng = &mut thread_rng();
/// let forest = &mut Rc::new(PrivateForest::new());
/// let root_dir = &mut Rc::new(PrivateDirectory::new(
/// Namefilter::default(),
/// Utc::now(),
/// rng,
/// ));
///
/// root_dir
/// .write(
/// &["code".into(), "python".into(), "hello.py".into()],
/// true,
/// Utc::now(),
/// b"print('hello world')".to_vec(),
/// forest,
/// store,
/// rng
/// )
/// .await
/// .unwrap();
///
/// let result = root_dir
/// .cp_link(
/// &["code".into(), "python".into(), "hello.py".into()],
/// &["code".into(), "hello.py".into()],
/// true,
/// forest,
/// store
/// )
/// .await
/// .unwrap();
///
/// let result = root_dir
/// .ls(&["code".into()], true, forest, store)
/// .await
/// .unwrap();
///
/// assert_eq!(result.len(), 2);
/// }
/// ```
#[allow(clippy::too_many_arguments)]
pub async fn cp_link(
self: &mut Rc<Self>,
path_segments_from: &[String],
path_segments_to: &[String],
search_latest: bool,
forest: &mut Rc<PrivateForest>,
store: &impl BlockStore,
) -> Result<()> {
let result = self
.get_node(path_segments_from, search_latest, forest, store)
.await?;

self.attach_link(
result.ok_or(FsError::NotFound)?,
path_segments_to,
search_latest,
forest,
store,
)
.await
}

/// Stores this PrivateDirectory in the PrivateForest.
///
/// # Examples
Expand Down
42 changes: 41 additions & 1 deletion wnfs/src/private/file.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use async_once_cell::OnceCell;
use async_stream::try_stream;
use chrono::{DateTime, Utc};
use futures::{future, AsyncRead, Stream, StreamExt, TryStreamExt};
use libipld::{Cid, IpldCodec};
use libipld::{Cid, Ipld, IpldCodec};
use rand_core::RngCore;
use semver::Version;
use serde::{de::Error as DeError, Deserialize, Deserializer, Serialize};
Expand Down Expand Up @@ -380,6 +380,35 @@ impl PrivateFile {
Ok(bytes)
}

/// Create a new Symlink PrivateFile
pub async fn new_symlink(
path: String,
parent_bare_name: Namefilter,
time: DateTime<Utc>,
rng: &mut impl RngCore,
) -> Result<Self> {
// Header stays the same
let header = PrivateNodeHeader::new(parent_bare_name, rng);
// Symlinks have no file content
let content = FileContent::Inline { data: vec![] };
// Create a new Metadata object
let mut metadata: Metadata = Metadata::new(time);
// Write the original path into the Metadata HashMap
metadata
.0
.insert(String::from("symlink"), Ipld::String(path));
// Return self with PrivateFileContent
Ok(Self {
header,
content: PrivateFileContent {
persisted_as: OnceCell::new(),
metadata,
previous: BTreeSet::new(),
content,
},
})
}
Comment on lines +383 to +410
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. I know you must have thought of making it a new node type. Is there a reason you chose this approach instead?


/// Gets the metadata of the file
pub fn get_metadata(&self) -> &Metadata {
&self.content.metadata
Expand Down Expand Up @@ -740,6 +769,17 @@ impl PrivateFile {
pub fn as_node(self: &Rc<Self>) -> PrivateNode {
PrivateNode::File(Rc::clone(self))
}

/// If the Metadata contains Symlink data, return it
pub fn symlink_origin(&self) -> Option<String> {
let meta = self.get_metadata();
// If the Metadata contains a String key for the symlink
if let Some(Ipld::String(path)) = meta.0.get("symlink") {
Some(path.to_string())
} else {
None
}
}
}

impl PrivateFileContent {
Expand Down