Skip to content

Add support for string config fields - #5

Open
ABuljko wants to merge 1 commit into
trussed-dev:mainfrom
ABuljko:config-string
Open

Add support for string config fields#5
ABuljko wants to merge 1 commit into
trussed-dev:mainfrom
ABuljko:config-string

Conversation

@ABuljko

@ABuljko ABuljko commented Aug 11, 2026

Copy link
Copy Markdown
  • Add FieldType::String and ConfigValueMut::String(&mut StringView)
  • Put both after U8, so Bool=0 and U8=1 stay the same
  • Keep FieldType::String a unit variant: the max length comes from the config struct, and values that are too long give DataTooLong
  • Take a StringView, so any String<N> works: hold a String<32> and pass as_mut_view()
  • Reject control characters, line and paragraph separators, and bidirectional embeddings, overrides and isolates with InvalidValue; the directional marks are still allowed
  • Check the length before clear(), so a rejected value keeps the old one
  • Enable the serde feature of heapless, needed for StringView's Serialize impl
  • Add tests for getting and setting a string field, both rejections, multi-byte values, and the field type ids

@robin-nitrokey robin-nitrokey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. Just one question regarding the forbidden characters:

Comment thread src/config.rs Outdated
@ABuljko
ABuljko marked this pull request as ready for review August 11, 2026 17:11

@robin-nitrokey robin-nitrokey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! Please remove the outdated comment and add an entry to the changelog.

Comment thread src/config.rs Outdated

@robin-nitrokey robin-nitrokey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! To do before merging:

  • squash commits
  • create draft PR for nitrokey-3-firmware to show how this is used

@ABuljko

ABuljko commented Aug 12, 2026

Copy link
Copy Markdown
Author

Should i temporarily link the nitrokey-3-firmware PR to my branch of the admin-app instead of main so that it can be tested properly

@robin-nitrokey

Copy link
Copy Markdown
Member

Yes, you can use a patch.crates-io section in Cargo.toml and point it to your branch or this PR.

- Add FieldType::String and ConfigValueMut::String, backed by a StringView so
  any String<N> can be used
- Put both after U8 so the existing type ids stay the same
- Take the max length from the config struct; too long gives DataTooLong
- Check the length before clear() so a rejected value keeps the old one
- Store arbitrary UTF-8; the client sanitizes the value before displaying it
- Enable the serde feature of heapless, needed for StringView's Serialize impl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants