Skip to content

Commit

Permalink
read_dir_first: stop at the first file that is alphabetically "after"…
Browse files Browse the repository at this point in the history
… `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).
  • Loading branch information
sosthene-nitrokey committed Feb 2, 2024
1 parent 2583e09 commit 59a51a2
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 2 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,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 = "77a45fa38f8bd0dd0197cce7735f43e9340fa3c6" }
7 changes: 6 additions & 1 deletion src/store/filestore.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use core::cmp::Ordering;

use crate::{
error::{Error, Result},
// service::ReadDirState,
Expand Down Expand Up @@ -164,7 +166,10 @@ impl<S: Store> ClientFilestore<S> {
// 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()
match entry.file_name().cmp_str(not_before) {
Ordering::Less => false,
Ordering::Equal | Ordering::Greater => true,
}
} else {
true
}
Expand Down
82 changes: 82 additions & 0 deletions tests/filesystem.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#![cfg(feature = "virt")]

use std::assert_eq;

use trussed::{
client::{CryptoClient, FilesystemClient},
error::Error,
Expand Down Expand Up @@ -85,15 +87,95 @@ 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.clone();
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.clone();
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(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(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());
});
}

#[test]
fn iterating_internal() {
iterating(Location::Internal);
iterating_first(Location::Internal);
}
#[test]
fn iterating_external() {
iterating(Location::External);
iterating_first(Location::Internal);
}
#[test]
fn iterating_volatile() {
iterating(Location::Volatile);
iterating_first(Location::Internal);
}

0 comments on commit 59a51a2

Please sign in to comment.