[PM-33496] Loosen parsing rules by introducing a new `Expected` value type by rbartlensky · Pull Request #127 · bitwarden/credential-exchange

rbartlensky · 2026-03-12T18:45:32Z

🎟️ Tracking

Solves #122

📔 Objective

Follow-up from our discussion on #126. Let me know what you think!

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

bitwarden-bot · 2026-03-12T18:46:22Z

Thank you for your contribution! We've added this to our internal tracking system for review.
ID: PM-33496
Link: https://bitwarden.atlassian.net/browse/PM-33496

Details on our contribution process can be found here: https://contributing.bitwarden.com/contributing/pull-requests/community-pr-process.

rbartlensky · 2026-03-13T12:01:18Z

I pushed two more commits which add a bunch of nice utilities to make upgrading to this new version easier. I ran out of time yesterday, so I spent today integrating these new patches into our codebase. Let me know what you think!

Hinton

I think this PR makes a lot of sense, I should update our usage and confirm it works well.

rbartlensky · 2026-03-25T13:35:05Z

Since this PR went through a few versions, I am going to clean up the history a bit once you're happy with the code.

Hinton · 2026-03-31T09:00:48Z

Since this PR went through a few versions, I am going to clean up the history a bit once you're happy with the code.

It will all be squashed in the end so no strict need to do that.

Hinton · 2026-03-31T17:06:23Z

I ran out of time reviewing this today. Overall I like the concept but it's a bit verbose and slightly clunky due to handling of valid but wrong field type. I'm experimenting with passing in the editable field type in the UnexpectedField so we avoid duplicating the inner data again. But not quite happy with the result yet.

Progdrasil

Just a small nit, otherwise LGTM

Hinton

Apologies for the slow review cycle. Overall I quite like this approach, thank you for continuing to iterate on it. I think we're duplicating things and I've suggested some improvements which I think will reduce the duplicated logic. Let me know what you think!

rbartlensky · 2026-04-13T13:49:53Z

@Hinton sorry for the long turnaround time -- I think I managed to address most of your comments. I will rebase once you're happy with the current impl

Hinton

Apologies for the delay.

This looks good, we should rebase it and I think we can merge it. It's somewhat annoying that we have two deserialize one for EditableFieldValue and one for EditableField but there doesn't seem to be any good way of eliminating it.

This helps us with parsing non-compliant CXF (for instance when someone passes in `string` instead of `concealed-string`) while also retaining type safety and compliant exports.

Hinton

I think we should add email and number since those got added to main? Looks good otherwise.

rbartlensky · 2026-04-23T14:52:54Z

@Hinton I think all your comments are now addressed :)

Hinton

I'll do quick smoke test of this in our implementation tomorrow but should be good to go.

Thanks for working on this!

rbartlensky · 2026-04-23T15:23:15Z

I'll do quick smoke test of this in our implementation tomorrow but should be good to go.

Good idea, I'm taking this for a spin as well on our side

Thanks for working on this!

Thanks for bearing with me -- I'm grateful for all the reviews you left on my patches!

rbartlensky · 2026-04-23T15:56:17Z

@Hinton I found one thing that made my life a lot easier: a conversion from editable field to string. It looks like it's working as expected on my side :)

codecov · 2026-04-24T14:24:15Z

Codecov Report

❌ Patch coverage is 57.61905% with 89 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.04%. Comparing base (404693f) to head (48c0cbc).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
credential-exchange-format/src/editable_field.rs	55.94%	89 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #127      +/-   ##
==========================================
- Coverage   66.02%   62.04%   -3.98%     
==========================================
  Files           5        5              
  Lines         518      685     +167     
==========================================
+ Hits          342      425      +83     
- Misses        176      260      +84

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

rbartlensky requested a review from a team as a code owner March 12, 2026 18:45

bitwarden-bot changed the title ~~Loosen parsing rules by introducing a new Expected value type~~ [PM-33496] Loosen parsing rules by introducing a new Expected value type Mar 12, 2026

bitwarden-bot added needs-qa community-pr labels Mar 12, 2026

rbartlensky mentioned this pull request Mar 12, 2026

[PM-33358] Loosen parsing strictness for some field types #126

Closed

rbartlensky commented Mar 12, 2026

View reviewed changes

Comment thread credential-exchange-format/src/editable_field.rs Outdated

Hinton self-assigned this Mar 16, 2026

Hinton reviewed Mar 17, 2026

View reviewed changes

Comment thread credential-exchange-format/src/document.rs

Comment thread credential-exchange-format/src/editable_field.rs

Comment thread credential-exchange-format/src/editable_field.rs

Progdrasil reviewed Mar 31, 2026

View reviewed changes

Comment thread credential-exchange-format/src/editable_field.rs Outdated

Hinton reviewed Apr 2, 2026

View reviewed changes

Hinton reviewed Apr 21, 2026

View reviewed changes

Comment thread credential-exchange-format/CHANGELOG.md Outdated

Comment thread credential-exchange-format/src/editable_field.rs

Comment thread credential-exchange-format/src/editable_field.rs Outdated

Add a new struct which encodes editable field expectations

5b4b6d5

This helps us with parsing non-compliant CXF (for instance when someone passes in `string` instead of `concealed-string`) while also retaining type safety and compliant exports.

rbartlensky force-pushed the expected-struct branch from 421aa4c to 5b4b6d5 Compare April 21, 2026 12:55

Hinton reviewed Apr 23, 2026

View reviewed changes

Comment thread credential-exchange-format/src/editable_field.rs

Comment thread credential-exchange-format/src/editable_field.rs Outdated

rbartlensky added 5 commits April 23, 2026 14:38

Add support for Email and Number fields

6c7d1e2

Add From impl for EditableFieldWifiNetworkSecurityType

e426f8c

Add From impl for EditableFieldBoolean

b309eb8

Add From impl for EditableFieldDate

f050b9c

Solve some clippy lints in tests

2555193

Hinton previously approved these changes Apr 23, 2026

View reviewed changes

Add to string conversion

48c0cbc

rbartlensky dismissed Hinton’s stale review via 48c0cbc April 23, 2026 15:55

Hinton approved these changes Apr 24, 2026

View reviewed changes

Hinton merged commit 482506e into bitwarden:main Apr 24, 2026
11 of 12 checks passed

rbartlensky deleted the expected-struct branch April 25, 2026 19:14

Hinton mentioned this pull request May 11, 2026

Handle wrong EditableField #122

Closed

Uh oh!

Conversation

rbartlensky commented Mar 12, 2026

🎟️ Tracking

📔 Objective

⏰ Reminders before review

🦮 Reviewer guidelines

Uh oh!

bitwarden-bot commented Mar 12, 2026

Uh oh!

Uh oh!

rbartlensky commented Mar 13, 2026

Uh oh!

Hinton left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rbartlensky commented Mar 25, 2026

Uh oh!

Hinton commented Mar 31, 2026

Uh oh!

Hinton commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Progdrasil left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Hinton left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rbartlensky commented Apr 13, 2026

Uh oh!

Hinton left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Hinton left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rbartlensky commented Apr 23, 2026

Uh oh!

Hinton left a comment

Choose a reason for hiding this comment

Uh oh!

rbartlensky commented Apr 23, 2026

Uh oh!

rbartlensky commented Apr 23, 2026

Uh oh!

codecov Bot commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Hinton left a comment •

edited

Loading

Hinton commented Mar 31, 2026 •

edited

Loading

Hinton left a comment •

edited

Loading

Hinton left a comment •

edited

Loading

Hinton left a comment •

edited

Loading

codecov Bot commented Apr 24, 2026 •

edited

Loading