Normalize entry parsing and display order

This commit is contained in:
2026-08-11 07:28:15 +02:00
parent 2a8c941a65
commit d21f5bd467
6 changed files with 262 additions and 74 deletions

View File

@@ -25,7 +25,7 @@ pub enum FieldNavigation {
impl EntryEditor {
pub fn new(document: EntryDocument) -> Self {
let focused = document.fields().first().map(EntryField::id);
let focused = document.display_fields().first().map(|field| field.id());
let multiline = document
.fields()
.iter()
@@ -86,7 +86,7 @@ impl EntryEditor {
}
pub fn focused_ratio(&self) -> f32 {
let fields = self.document.fields();
let fields = self.document.display_fields();
self.focused
.and_then(|id| fields.iter().position(|field| field.id() == id))
.map_or(0.0, |index| {
@@ -128,7 +128,7 @@ impl EntryEditor {
}
pub fn navigate(&mut self, navigation: FieldNavigation) {
let fields = self.document.fields();
let fields = self.document.display_fields();
if fields.is_empty() {
self.focused = None;
return;

View File

@@ -4945,7 +4945,6 @@ fn viewer_label(field: &EntryField) -> String {
EntryFieldKind::OtpUri => "One-time password".to_owned(),
EntryFieldKind::Field => "Field".to_owned(),
EntryFieldKind::Note => "Note".to_owned(),
EntryFieldKind::Blank => "Blank line".to_owned(),
},
str::to_owned,
)
@@ -4982,7 +4981,7 @@ fn viewer_view<'a>(app: &'a App, editor: &'a EntryEditor) -> Element<'a, Message
column![text("Up/Down/Home/End select fields · ⌘C copies the selected value").size(12),]
.spacing(8);
for field in editor.fields() {
for field in editor.document().display_fields() {
let id = field.id();
let label = viewer_label(field);
let selected = editor.focused() == Some(id);
@@ -5943,7 +5942,7 @@ mod tests {
let temporary = editor.fields().last().expect("temporary field").id();
editor.remove(temporary).expect("remove temporary line");
let expected =
b"password\nusername: alice\nfirst note line\ncustom-field: opaque\nsecond note line";
b"password\nusername: alice\n first note line\ncustom-field: opaque\n second note line";
assert_eq!(editor.document().serialize().expose(), expected);
save_document(&storage, &editor).expect("initial save");
@@ -5966,7 +5965,7 @@ mod tests {
assert!(stale.is_dirty());
assert_eq!(
stale.document().serialize().expose(),
b"complete stale draft\nusername: alice\nfirst note line\ncustom-field: opaque\nsecond note line"
b"complete stale draft\nusername: alice\n first note line\ncustom-field: opaque\n second note line"
);
}
@@ -6057,12 +6056,32 @@ mod tests {
.text()
);
assert!(document_has_single_totp(editor.document()));
assert_eq!(
editor
.document()
.display_fields()
.into_iter()
.map(EntryField::id)
.collect::<Vec<_>>(),
[
editor.fields()[1].id(),
editor.fields()[0].id(),
editor.fields()[7].id(),
editor.fields()[8].id(),
editor.fields()[4].id(),
editor.fields()[5].id(),
editor.fields()[6].id(),
editor.fields()[9].id(),
editor.fields()[2].id(),
editor.fields()[3].id(),
]
);
editor.navigate(FieldNavigation::First);
assert_eq!(editor.focused(), Some(password));
editor.navigate(FieldNavigation::Next);
assert_eq!(editor.focused(), Some(editor.fields()[1].id()));
editor.navigate(FieldNavigation::Next);
assert_eq!(editor.focused(), Some(password));
editor.navigate(FieldNavigation::Last);
assert_eq!(editor.focused(), Some(editor.fields()[9].id()));
assert_eq!(editor.focused(), Some(editor.fields()[3].id()));
assert!(!authentication_allows_content(&AuthenticationView::Locked));
assert!(authentication_allows_content(

View File

@@ -713,17 +713,17 @@ fn viewer_lines<'a>(
otp_display: Option<&'a crate::app::OtpDisplay>,
) -> Vec<Line<'a>> {
let focused = viewer.focused_index();
if viewer.document().fields().is_empty() {
let fields = viewer.document().display_fields();
if fields.is_empty() {
return vec![Line::from("This entry is empty.")];
}
let mut lines = Vec::new();
for (index, field) in viewer.document().fields().iter().enumerate() {
for (index, field) in fields.into_iter().enumerate() {
let metadata = field.metadata();
let label = metadata.name().map_or_else(
|| match metadata.kind() {
ironstorage::document::EntryFieldKind::Note => format!("note {}", index + 1),
ironstorage::document::EntryFieldKind::Blank => format!("blank {}", index + 1),
kind => format!("{kind:?}").to_ascii_lowercase(),
},
str::to_owned,
@@ -1514,14 +1514,19 @@ mod tests {
render(40, 8, &app);
assert_eq!(app.viewer().expect("viewer").scroll(), 0);
app.dispatch(crate::action::Action::Next);
app.dispatch(crate::action::Action::FocusNext);
app.dispatch(crate::action::Action::FocusNext);
let wrapped = render(40, 8, &app);
assert!(wrapped.contains("url: xxxxxx"));
let wrapped_scroll = app.viewer().expect("viewer").scroll();
assert_eq!(wrapped_scroll, 0);
app.dispatch(crate::action::Action::FocusNext);
let login = render(40, 8, &app);
assert!(login.contains("login: alice@example.test"));
render(40, 8, &app);
assert_eq!(
app.viewer()
.and_then(crate::viewer::EntryViewer::focused_field)
.map(|field| field.metadata().kind()),
Some(ironstorage::document::EntryFieldKind::Url)
);
assert!(app.viewer().expect("viewer").scroll() > wrapped_scroll);
render(140, 20, &app);
@@ -1583,6 +1588,7 @@ mod tests {
let mut app = App::new();
app.open_test_document("documents/multiline", document);
app.dispatch(crate::action::Action::FocusNext);
app.dispatch(crate::action::Action::FocusNext);
let viewer = app.viewer().expect("viewer");
assert_eq!(
@@ -1591,7 +1597,7 @@ mod tests {
);
let lines = viewer_lines(viewer, None);
assert_eq!(lines.len(), 5);
for line in &lines[1..4] {
for line in &lines[2..5] {
assert!(line.spans.iter().all(|span| {
span.style.fg == Some(SELECTED_FOREGROUND)
&& span.style.bg == Some(SELECTED_BACKGROUND)

View File

@@ -39,7 +39,7 @@ impl EntryViewer {
}
pub fn focused_field(&self) -> Option<&EntryField> {
self.document.fields().get(self.focused)
self.document.display_fields().get(self.focused).copied()
}
pub fn focus_next(&mut self) {
@@ -231,8 +231,7 @@ mod tests {
#[test]
fn copy_uses_only_the_focused_structured_value() {
let mut viewer = EntryViewer::new(fixture_document("email/personal"));
viewer.focus_next();
let viewer = EntryViewer::new(fixture_document("email/personal"));
let copied = viewer.copy_focused().expect("copy value");
assert_eq!(copied.expose(), b"alice@example.test");
assert!(!format!("{viewer:?}").contains("correct horse fixture"));

View File

@@ -31,7 +31,6 @@ pub enum EntryFieldKind {
OtpUri,
Field,
Note,
Blank,
}
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
@@ -234,6 +233,7 @@ pub struct EntryDocument {
next_id: u64,
conflict_token: DocumentConflictToken,
session: EditSession,
modified: bool,
}
impl EntryDocument {
@@ -249,6 +249,7 @@ impl EntryDocument {
next_id,
conflict_token,
session,
modified: false,
}
}
@@ -260,6 +261,13 @@ impl EntryDocument {
&self.fields
}
/// Fields in the shared presentation order without changing repository order.
pub fn display_fields(&self) -> Vec<&EntryField> {
let mut fields = self.fields.iter().collect::<Vec<_>>();
fields.sort_by_cached_key(|field| display_key(field));
fields
}
pub fn field(&self, id: EntryFieldId) -> Option<&EntryField> {
self.fields.iter().find(|field| field.id == id)
}
@@ -289,7 +297,7 @@ impl EntryDocument {
let draft = match field.metadata().kind() {
EntryFieldKind::OtpUri => EntryFieldDraft::otp_uri(value)?,
EntryFieldKind::Password => EntryFieldDraft::line(value)?,
EntryFieldKind::Note | EntryFieldKind::Blank => EntryFieldDraft::multiline(value),
EntryFieldKind::Note => EntryFieldDraft::multiline(value),
EntryFieldKind::Username
| EntryFieldKind::Email
| EntryFieldKind::Url
@@ -329,11 +337,12 @@ impl EntryDocument {
id,
contents: draft.render(),
ending: default_ending.clone(),
metadata: blank_metadata(),
metadata: empty_note_metadata(),
},
);
normalize_endings(&mut self.fields, &default_ending, final_newline);
classify_all(&mut self.fields);
self.modified = true;
Ok(id)
}
@@ -342,13 +351,14 @@ impl EntryDocument {
id: EntryFieldId,
draft: EntryFieldDraft,
) -> Result<(), DocumentError> {
let field = self
let index = self
.fields
.iter_mut()
.find(|field| field.id == id)
.iter()
.position(|field| field.id == id)
.ok_or(DocumentError::UnknownField { id })?;
field.contents = draft.render();
self.fields[index].contents = draft.render();
classify_all(&mut self.fields);
self.modified = true;
Ok(())
}
@@ -363,6 +373,7 @@ impl EntryDocument {
let removed = self.fields.remove(index);
normalize_endings(&mut self.fields, &default_ending, final_newline);
classify_all(&mut self.fields);
self.modified = true;
Ok(removed)
}
@@ -384,10 +395,14 @@ impl EntryDocument {
self.fields.insert(index, field);
normalize_endings(&mut self.fields, &default_ending, final_newline);
classify_all(&mut self.fields);
self.modified = true;
Ok(())
}
pub fn serialize(&self) -> SecretBytes {
if !self.modified {
return SecretBytes::new(self.session.plaintext().expose().to_vec());
}
let capacity = self
.fields
.iter()
@@ -395,7 +410,7 @@ impl EntryDocument {
.sum();
let mut output = Vec::with_capacity(capacity);
for field in &self.fields {
output.extend_from_slice(field.contents.expose());
append_stable_contents(&mut output, field);
output.extend_from_slice(&field.ending);
}
SecretBytes::new(output)
@@ -540,7 +555,7 @@ fn parse_lines(contents: &[u8]) -> Vec<EntryField> {
id: EntryFieldId(fields.len() as u64),
contents: SecretBytes::new(contents[start..content_end].to_vec()),
ending,
metadata: blank_metadata(),
metadata: empty_note_metadata(),
});
start = end;
}
@@ -551,7 +566,12 @@ fn parse_lines(contents: &[u8]) -> Vec<EntryField> {
&& logical.last().is_some_and(|previous| {
let metadata =
classify(logical.len().saturating_sub(1), previous.contents.expose());
metadata.name.is_some() && metadata.kind != EntryFieldKind::OtpUri
metadata.kind == EntryFieldKind::Note
|| (metadata.name.is_some()
&& !matches!(
metadata.kind,
EntryFieldKind::Password | EntryFieldKind::OtpUri
))
});
if continuation {
let previous = logical
@@ -559,10 +579,16 @@ fn parse_lines(contents: &[u8]) -> Vec<EntryField> {
.expect("continuation has a previous field");
let mut contents = previous.contents.expose().to_vec();
contents.extend_from_slice(&previous.ending);
contents.extend_from_slice(field.contents.expose());
contents.extend_from_slice(strip_continuation_marker(field.contents.expose()));
previous.contents = SecretBytes::new(contents);
previous.ending = field.ending;
} else {
let mut field = field;
if classify(logical.len(), field.contents.expose()).kind == EntryFieldKind::Note
&& let Some(contents) = field.contents.expose().strip_prefix(b" ")
{
field.contents = SecretBytes::new(contents.to_vec());
}
logical.push(field);
}
}
@@ -612,7 +638,7 @@ fn classify(index: usize, line: &[u8]) -> EntryFieldMetadata {
};
}
if line.is_empty() {
return blank_metadata();
return empty_note_metadata();
}
if line.starts_with(b"otpauth://") {
return EntryFieldMetadata {
@@ -629,24 +655,22 @@ fn classify(index: usize, line: &[u8]) -> EntryFieldMetadata {
.position(|byte| *byte == b'\n')
.unwrap_or(line.len());
let first_line = &line[..first_line_end];
if let Some(colon) = first_line.iter().position(|byte| *byte == b':') {
let raw_name = trim_ascii(&first_line[..colon]);
if !raw_name.is_empty()
&& let Ok(name) = std::str::from_utf8(raw_name)
{
let value_start = colon + 1 + usize::from(line.get(colon + 1) == Some(&b' '));
let kind = semantic_field_kind(name);
return EntryFieldMetadata {
kind,
sensitivity: field_sensitivity(name, kind),
name: Some(name.to_owned()),
otp: None,
diagnostic: std::str::from_utf8(&line[value_start..])
.is_err()
.then_some(EntryFieldDiagnostic::NonUtf8Value),
value: value_start..line.len(),
};
}
if let Some(colon) = first_line.iter().position(|byte| *byte == b':')
&& let Ok(name) = std::str::from_utf8(&first_line[..colon])
&& is_field_name(name)
{
let value_start = colon + 1 + usize::from(line.get(colon + 1) == Some(&b' '));
let kind = semantic_field_kind(name);
return EntryFieldMetadata {
kind,
sensitivity: field_sensitivity(name, kind),
name: Some(name.to_owned()),
otp: None,
diagnostic: std::str::from_utf8(&line[value_start..])
.is_err()
.then_some(EntryFieldDiagnostic::NonUtf8Value),
value: value_start..line.len(),
};
}
EntryFieldMetadata {
kind: EntryFieldKind::Note,
@@ -661,7 +685,7 @@ fn classify(index: usize, line: &[u8]) -> EntryFieldMetadata {
}
fn semantic_field_kind(name: &str) -> EntryFieldKind {
match name.trim().to_ascii_lowercase().as_str() {
match name.to_ascii_lowercase().as_str() {
"user" | "username" | "login" => EntryFieldKind::Username,
"email" | "e-mail" => EntryFieldKind::Email,
"url" | "uri" | "website" => EntryFieldKind::Url,
@@ -676,15 +700,31 @@ fn field_sensitivity(name: &str, kind: EntryFieldKind) -> EntrySensitivity {
) {
return EntrySensitivity::Ordinary;
}
match name.trim().to_ascii_lowercase().as_str() {
match name.to_ascii_lowercase().as_str() {
"title" | "site" | "host" | "autotype_enabled" | "icon" => EntrySensitivity::Ordinary,
_ => EntrySensitivity::Sensitive,
}
}
fn blank_metadata() -> EntryFieldMetadata {
fn display_key(field: &EntryField) -> (u8, String) {
let metadata = field.metadata();
let name = metadata.name().unwrap_or_default().to_lowercase();
let priority = match metadata.kind() {
EntryFieldKind::Username => 0,
EntryFieldKind::Password => 1,
EntryFieldKind::Url => 2,
EntryFieldKind::OtpUri => 3,
EntryFieldKind::Note => 4,
_ if matches!(name.as_str(), "note" | "notes") => 4,
_ if name == "comments" => 5,
EntryFieldKind::Email | EntryFieldKind::Field => 6,
};
(priority, if priority == 6 { name } else { String::new() })
}
fn empty_note_metadata() -> EntryFieldMetadata {
EntryFieldMetadata {
kind: EntryFieldKind::Blank,
kind: EntryFieldKind::Note,
sensitivity: EntrySensitivity::Empty,
name: None,
otp: None,
@@ -693,16 +733,6 @@ fn blank_metadata() -> EntryFieldMetadata {
}
}
fn trim_ascii(mut value: &[u8]) -> &[u8] {
while value.first().is_some_and(u8::is_ascii_whitespace) {
value = &value[1..];
}
while value.last().is_some_and(u8::is_ascii_whitespace) {
value = &value[..value.len() - 1];
}
value
}
fn validate_line(value: &[u8]) -> Result<(), DocumentError> {
if value.iter().any(|byte| matches!(byte, b'\r' | b'\n')) {
Err(DocumentError::LineBreak)
@@ -712,13 +742,33 @@ fn validate_line(value: &[u8]) -> Result<(), DocumentError> {
}
fn validate_name(name: &str) -> Result<(), DocumentError> {
if name.trim().is_empty() || name.contains([':', '\r', '\n']) || name.trim() != name {
if !is_field_name(name) {
Err(DocumentError::InvalidFieldName)
} else {
Ok(())
}
}
fn is_field_name(name: &str) -> bool {
!name.is_empty() && !name.contains([':', '\r', '\n']) && !name.chars().any(char::is_whitespace)
}
fn strip_continuation_marker(contents: &[u8]) -> &[u8] {
contents.strip_prefix(b" ").unwrap_or(contents)
}
fn append_stable_contents(output: &mut Vec<u8>, field: &EntryField) {
if field.metadata().kind() == EntryFieldKind::Note {
output.push(b' ');
}
for byte in field.contents.expose() {
output.push(*byte);
if *byte == b'\n' {
output.push(b' ');
}
}
}
fn normalize_endings(fields: &mut [EntryField], default: &[u8], final_newline: bool) {
let last = fields.len().saturating_sub(1);
for (index, field) in fields.iter_mut().enumerate() {

View File

@@ -79,7 +79,7 @@ fn complex_documents_round_trip_with_storage_owned_metadata() -> TestResult {
document.password().expect("password").value(),
"pässwörd".as_bytes()
);
assert_eq!(document.fields().len(), 11);
assert_eq!(document.fields().len(), 9);
let kinds = document
.fields()
@@ -96,8 +96,6 @@ fn complex_documents_round_trip_with_storage_owned_metadata() -> TestResult {
EntryFieldKind::Field,
EntryFieldKind::Field,
EntryFieldKind::OtpUri,
EntryFieldKind::Blank,
EntryFieldKind::Note,
EntryFieldKind::Note,
EntryFieldKind::Field,
]
@@ -123,11 +121,15 @@ fn complex_documents_round_trip_with_storage_owned_metadata() -> TestResult {
assert_eq!(document.fields()[5].metadata().name(), Some("custom"));
assert!(document.fields()[5].value().is_empty());
assert_ne!(document.fields()[4].id(), document.fields()[5].id());
assert_eq!(document.fields()[10].metadata().name(), Some(""));
assert_eq!(document.fields()[8].metadata().name(), Some(""));
assert_eq!(
document.fields()[10].value(),
document.fields()[8].value(),
"\r\nunrecognized line".as_bytes()
);
assert_eq!(
document.fields()[7].value(),
b"\r\nfirst note\r\nsecond note"
);
assert_eq!(
document.fields()[6].metadata().sensitivity(),
EntrySensitivity::Sensitive
@@ -175,7 +177,119 @@ fn named_multiline_fields_are_one_lossless_logical_field() -> TestResult {
document.replace_field_value(comments.id(), b"New heading\nalpha\nbeta".to_vec())?;
assert_eq!(
document.serialize().expose(),
b"password\ncomments: New heading\nalpha\nbeta\nurl: https://example.test\n"
b"password\ncomments: New heading\n alpha\n beta\nurl: https://example.test\n"
);
Ok(())
}
#[test]
fn only_whitespace_free_names_end_multiline_fields() -> TestResult {
let fixture = FixtureSet::load()?;
let store = fixture.materialize_store("basic")?;
let repository = Repository::open(store.path())?;
let keys = KeyStore::load(fixture.path("keys"))?;
let mut secrets = FixtureSecrets::all(&fixture);
let plaintext = "password\ncomments: PIN: 5678\nVertragsnummer 1234\n\nwww.bahn.de Login:\n\nbenutzer / passwort\n\n(über Geschäftskundenlogin gehen!)\n\nautotype_enabled: True\nicon: 0\n";
write_plaintext(
&repository,
&keys,
"documents/comments",
plaintext.as_bytes(),
)?;
let document =
EntryDocumentService::new(&repository, &keys).open("documents/comments", &mut secrets)?;
assert_eq!(document.serialize().expose(), plaintext.as_bytes());
assert_eq!(document.fields().len(), 4);
assert_eq!(document.fields()[1].metadata().name(), Some("comments"));
assert_eq!(
document.fields()[1].value(),
"PIN: 5678\nVertragsnummer 1234\n\nwww.bahn.de Login:\n\nbenutzer / passwort\n\n(über Geschäftskundenlogin gehen!)\n"
.as_bytes()
);
assert_eq!(
document.fields()[2].metadata().name(),
Some("autotype_enabled")
);
assert_eq!(document.fields()[3].metadata().name(), Some("icon"));
assert!(matches!(
EntryFieldDraft::field("www.bahn.de Login", Vec::new()),
Err(DocumentError::InvalidFieldName)
));
write_plaintext(
&repository,
&keys,
"documents/plain-note",
b"password\n\nordinary note\ncontinued",
)?;
let note =
EntryDocumentService::new(&repository, &keys).open("documents/plain-note", &mut secrets)?;
assert_eq!(note.password().expect("password").value(), b"password");
assert_eq!(note.fields().len(), 2);
assert_eq!(note.fields()[1].value(), b"\nordinary note\ncontinued");
let unordered = "password\nzeta: last\nnote: named\ncomments: heading\notpauth://totp/Example:alice?secret=JBSWY3DPEHPK3PXP&issuer=Example\nordinary note\nurl: https://example.test\nlogin: alice\nalpha: first\n";
write_plaintext(
&repository,
&keys,
"documents/unordered",
unordered.as_bytes(),
)?;
let mut unordered_document =
EntryDocumentService::new(&repository, &keys).open("documents/unordered", &mut secrets)?;
assert_eq!(
unordered_document.serialize().expose(),
unordered.as_bytes()
);
assert_eq!(
unordered_document
.display_fields()
.into_iter()
.map(|field| field.metadata().name().unwrap_or("note"))
.collect::<Vec<_>>(),
[
"login", "password", "url", "otp", "note", "note", "comments", "alpha", "zeta"
]
);
let comments = unordered_document
.fields()
.iter()
.find(|field| field.metadata().name() == Some("comments"))
.expect("comments")
.id();
unordered_document
.replace_field_value(comments, b"heading\nline: still a comment\n\nend".to_vec())?;
assert_eq!(
unordered_document.serialize().expose(),
b"password\nzeta: last\nnote: named\ncomments: heading\n line: still a comment\n \n end\notpauth://totp/Example:alice?secret=JBSWY3DPEHPK3PXP&issuer=Example\n ordinary note\nurl: https://example.test\nlogin: alice\nalpha: first\n"
);
let normalized = unordered_document.serialize();
write_plaintext(
&repository,
&keys,
"documents/normalized",
normalized.expose(),
)?;
let normalized_document =
EntryDocumentService::new(&repository, &keys).open("documents/normalized", &mut secrets)?;
assert_eq!(
normalized_document
.fields()
.iter()
.find(|field| field.metadata().name() == Some("comments"))
.expect("normalized comments")
.value(),
b"heading\nline: still a comment\n\nend"
);
assert_eq!(
normalized_document
.fields()
.iter()
.find(|field| field.metadata().kind() == EntryFieldKind::Note)
.expect("normalized note")
.value(),
b"ordinary note"
);
Ok(())
}
@@ -264,8 +378,8 @@ fn field_ids_survive_updates_removal_and_reordering() -> TestResult {
empty.add(0, EntryFieldDraft::line(b"password".to_vec())?)?;
empty.add(1, EntryFieldDraft::blank())?;
assert_eq!(empty.fields().len(), 2);
assert_eq!(empty.fields()[1].metadata().kind(), EntryFieldKind::Blank);
assert_eq!(empty.serialize().expose(), b"password\n");
assert_eq!(empty.fields()[1].metadata().kind(), EntryFieldKind::Note);
assert_eq!(empty.serialize().expose(), b"password\n ");
assert!(matches!(
EntryFieldDraft::line(b"two\nlines".to_vec()),