read_dir_first: stop at the first file that is alphabetically "after"…

… `not_before`

In fido-authenticator, if we change the paths of RK  to be:
"rp_id.rk_id" instead of the current "rp_id/rk_id", we still want to be able
to iterate over the keys even though we only  know the "rp_id" and not the
"rk_id". Therefore we need to be able to stop at "rp_id.***" when giving "rp_id" in `not_before`

This is technically a breaking change because now, given the files:

- "aaa"
- "aaabbb"

 "read_dir_first"  with "aaa" as `not_before` would only yield "aaa"
due to littlefs-project/littlefs#923.
Now this will yield both files, and yield "aaabbb" first.

I beleive this behaviour is technically more correct as it is likely what
would be expected to be yield expecting alphabetical order
(though the order of the entries is still incorrect).

Cargo.toml

@@ -144,4 +144,4 @@ features = ["serde-extensions", "virt"]
 rustdoc-args = ["--cfg", "docsrs"]
 
 [patch.crates-io]
-littlefs2 = { git = "https://github.com/trussed-dev/littlefs2.git", rev = "ebd27e49ca321089d01d8c9b169c4aeb58ceeeca" }
+littlefs2 = { git = "https://github.com/sosthene-nitrokey/littlefs2.git", rev = "2b45a7559ff44260c6dd693e4cb61f54ae5efc53" }

src/api.rs

@@ -244,7 +244,7 @@ pub mod request {
         ReadDirFirst:
           - location: Location
           - dir: PathBuf
-          - not_before_filename: Option<PathBuf>
+          - not_before: Option<(PathBuf, bool)>
 
         ReadDirNext:
 

src/client.rs

@@ -598,6 +598,9 @@ pub trait FilesystemClient: PollClient {
         self.request(request::DebugDumpStore {})
     }
 
+    /// Open a directory for iteration with `read_dir_next`
+    ///
+    /// For optimization, not_before_filename can be passed to begin the iteration at that file.
     fn read_dir_first(
         &mut self,
         location: Location,
@@ -607,7 +610,27 @@ pub trait FilesystemClient: PollClient {
         self.request(request::ReadDirFirst {
             location,
             dir,
-            not_before_filename,
+            not_before: not_before_filename.map(|p| (p, true)),
+        })
+    }
+
+    /// Open a directory for iteration with `read_dir_next`
+    ///
+    /// For optimization, not_before_filename can be passed to begin the iteration after the first file that is "alphabetically" before the original file
+    ///
+    /// <div class="warning">
+    /// The notion used here for "alphabetical" does not correspond to the order of iteration yielded by littlefs. This function should be used with caution. If `not_before_filename` was yielded from a previous use of read_dir, it can lead to entries being repeated.
+    /// </div>
+    fn read_dir_first_alphabetical(
+        &mut self,
+        location: Location,
+        dir: PathBuf,
+        not_before_filename: Option<PathBuf>,
+    ) -> ClientResult<'_, reply::ReadDirFirst, Self> {
+        self.request(request::ReadDirFirst {
+            location,
+            dir,
+            not_before: not_before_filename.map(|p| (p, false)),
         })
     }
 

src/service.rs

@@ -409,7 +409,10 @@ impl<P: Platform> ServiceResources<P> {
                 let maybe_entry = match filestore.read_dir_first(
                     &request.dir,
                     request.location,
-                    request.not_before_filename.as_deref(),
+                    request
+                        .not_before
+                        .as_ref()
+                        .map(|(path, require_equal)| (&**path, *require_equal)),
                 )? {
                     Some((entry, read_dir_state)) => {
                         ctx.read_dir_state = Some(read_dir_state);

src/store/filestore.rs

@@ -1,3 +1,5 @@
+use core::cmp::Ordering;
+
 use crate::{
     error::{Error, Result},
     // service::ReadDirState,
@@ -117,7 +119,7 @@ pub trait Filestore {
         &mut self,
         dir: &Path,
         location: Location,
-        not_before: Option<&Path>,
+        not_before: Option<(&Path, bool)>,
     ) -> Result<Option<(DirEntry, ReadDirState)>>;
 
     /// Continue iterating over entries of a directory.
@@ -153,7 +155,7 @@ impl<S: Store> ClientFilestore<S> {
         &mut self,
         clients_dir: &Path,
         location: Location,
-        not_before: Option<&Path>,
+        not_before: Option<(&Path, bool)>,
     ) -> Result<Option<(DirEntry, ReadDirState)>> {
         let fs = self.store.fs(location);
         let dir = self.actual_path(clients_dir)?;
@@ -171,8 +173,15 @@ impl<S: Store> ClientFilestore<S> {
                     .map(|(i, entry)| (i, entry.unwrap()))
                     // if there is a "not_before" entry, skip all entries before it.
                     .find(|(_, entry)| {
-                        if let Some(not_before) = not_before {
-                            entry.file_name() == not_before.as_ref()
+                        if let Some((not_before, require_equal)) = not_before {
+                            if require_equal {
+                                entry.file_name() == not_before
+                            } else {
+                                match entry.file_name().cmp_str(not_before) {
+                                    Ordering::Less => false,
+                                    Ordering::Equal | Ordering::Greater => true,
+                                }
+                            }
                         } else {
                             true
                         }
@@ -437,7 +446,7 @@ impl<S: Store> Filestore for ClientFilestore<S> {
         &mut self,
         clients_dir: &Path,
         location: Location,
-        not_before: Option<&Path>,
+        not_before: Option<(&Path, bool)>,
     ) -> Result<Option<(DirEntry, ReadDirState)>> {
         self.read_dir_first_impl(clients_dir, location, not_before)
     }

tests/filesystem.rs

@@ -1,5 +1,7 @@
 #![cfg(feature = "virt")]
 
+use std::assert_eq;
+
 use trussed::{
     client::{CryptoClient, FilesystemClient},
     error::Error,
@@ -85,15 +87,217 @@ fn iterating(location: Location) {
     });
 }
 
+fn iterating_first(location: Location) {
+    use littlefs2::path;
+    client::get(|client| {
+        let files = [
+            path!("foo"),
+            path!("bar"),
+            path!("baz"),
+            path!("foobar"),
+            path!("foobaz"),
+        ];
+
+        let files_sorted_lfs = {
+            let mut files = files;
+            files.sort_by(|a, b| a.cmp_lfs(b));
+            files
+        };
+
+        assert_eq!(
+            files_sorted_lfs,
+            [
+                path!("bar"),
+                path!("baz"),
+                path!("foobar"),
+                path!("foobaz"),
+                path!("foo"),
+            ]
+        );
+
+        let files_sorted_str = {
+            let mut files = files;
+            files.sort_by(|a, b| a.cmp_str(b));
+            files
+        };
+        assert_eq!(
+            files_sorted_str,
+            [
+                path!("bar"),
+                path!("baz"),
+                path!("foo"),
+                path!("foobar"),
+                path!("foobaz"),
+            ]
+        );
+
+        for f in files {
+            syscall!(client.write_file(
+                location,
+                PathBuf::from(f),
+                Bytes::from_slice(f.as_ref().as_bytes()).unwrap(),
+                None
+            ));
+        }
+
+        let first_entry =
+            syscall!(client.read_dir_first_alphabetical(location, PathBuf::from(""), None))
+                .entry
+                .unwrap();
+        assert_eq!(first_entry.path(), files_sorted_lfs[0]);
+        for f in &files_sorted_lfs[1..] {
+            let entry = syscall!(client.read_dir_next()).entry.unwrap();
+            assert_eq!(&entry.path(), f);
+        }
+        assert!(syscall!(client.read_dir_next()).entry.is_none());
+
+        let first_entry = syscall!(client.read_dir_first_alphabetical(
+            location,
+            PathBuf::from(""),
+            Some(PathBuf::from("fo"))
+        ))
+        .entry
+        .unwrap();
+        assert_eq!(first_entry.path(), path!("foobar"));
+
+        for f in &(files_sorted_lfs[3..]) {
+            let entry = syscall!(client.read_dir_next()).entry.unwrap();
+            assert_eq!(&entry.path(), f);
+        }
+        assert!(syscall!(client.read_dir_next()).entry.is_none());
+    });
+}
+
+fn iterating_files_and_dirs(location: Location) {
+    use littlefs2::path;
+    client::get(|client| {
+        let files = [
+            path!("foo"),
+            path!("bar"),
+            path!("baz"),
+            path!("foobar"),
+            path!("foobaz"),
+        ];
+
+        for f in files {
+            syscall!(client.write_file(
+                location,
+                PathBuf::from(f),
+                Bytes::from_slice(f.as_ref().as_bytes()).unwrap(),
+                None
+            ));
+        }
+
+        let directories = [
+            path!("dir"),
+            path!("foodir"),
+            path!("bardir"),
+            path!("bazdir"),
+            path!("foobardir"),
+            path!("foobazdir"),
+        ];
+
+        for d in directories {
+            let mut file_path = PathBuf::from(d);
+            file_path.push(path!("file"));
+
+            syscall!(client.write_file(
+                location,
+                file_path.clone(),
+                Bytes::from_slice(file_path.as_ref().as_bytes()).unwrap(),
+                None
+            ));
+        }
+
+        let all_entries: Vec<_> = files.into_iter().chain(directories).collect();
+        let all_entries_sorted_str = {
+            let mut all_entries = all_entries.clone();
+            all_entries.sort_by(|a, b| a.cmp_str(b));
+            all_entries
+        };
+
+        assert_eq!(
+            all_entries_sorted_str,
+            [
+                path!("bar"),
+                path!("bardir"),
+                path!("baz"),
+                path!("bazdir"),
+                path!("dir"),
+                path!("foo"),
+                path!("foobar"),
+                path!("foobardir"),
+                path!("foobaz"),
+                path!("foobazdir"),
+                path!("foodir"),
+            ]
+        );
+
+        let all_entries_sorted_lfs = {
+            let mut all_entries = all_entries.clone();
+            all_entries.sort_by(|a, b| a.cmp_lfs(b));
+            all_entries
+        };
+
+        assert_eq!(
+            all_entries_sorted_lfs,
+            [
+                path!("bardir"),
+                path!("bar"),
+                path!("bazdir"),
+                path!("baz"),
+                path!("dir"),
+                path!("foobardir"),
+                path!("foobar"),
+                path!("foobazdir"),
+                path!("foobaz"),
+                path!("foodir"),
+                path!("foo"),
+            ]
+        );
+
+        let first_entry =
+            syscall!(client.read_dir_first_alphabetical(location, PathBuf::from(""), None))
+                .entry
+                .unwrap();
+        assert_eq!(first_entry.path(), all_entries_sorted_lfs[0]);
+        for f in &all_entries_sorted_lfs[1..] {
+            let entry = syscall!(client.read_dir_next()).entry.unwrap();
+            assert_eq!(&entry.path(), f);
+        }
+        assert!(syscall!(client.read_dir_next()).entry.is_none());
+
+        let first_entry = syscall!(client.read_dir_first_alphabetical(
+            location,
+            PathBuf::from(""),
+            Some(PathBuf::from("dir"))
+        ))
+        .entry
+        .unwrap();
+        assert_eq!(first_entry.path(), all_entries_sorted_lfs[4]);
+        for f in &all_entries_sorted_lfs[5..] {
+            let entry = syscall!(client.read_dir_next()).entry.unwrap();
+            assert_eq!(&entry.path(), f);
+        }
+        assert!(syscall!(client.read_dir_next()).entry.is_none());
+    });
+}
+
 #[test]
 fn iterating_internal() {
     iterating(Location::Internal);
+    iterating_first(Location::Internal);
+    iterating_files_and_dirs(Location::Internal);
 }
 #[test]
 fn iterating_external() {
     iterating(Location::External);
+    iterating_first(Location::External);
+    iterating_files_and_dirs(Location::External);
 }
 #[test]
 fn iterating_volatile() {
     iterating(Location::Volatile);
+    iterating_first(Location::Volatile);
+    iterating_files_and_dirs(Location::Volatile);
 }