Skip to content

Conversation

harr1424
Copy link

🎟️ Tracking

Web app PR

https://github.com/orgs/bitwarden/discussions/15942

https://community.bitwarden.com/t/duplicate-removal-tool-report-including-merge/648

📔 Objective

This pull request is intended to integrate logic for finding duplicate ciphers into the SDK. It is a best effort attempt to directly implement a TypeScript service used in the Web app into the SDK.

Limitations & Divergence from Web app

  • When duplicates are collected using the Host matching strategy, port numbers present in the URL that match the URL's scheme ( 80 for HTTP, 443 for HTTPS ) will not be considered. This is due to a differences in the npm URL API and the url crate available in Rust. In the future, regular expressions could likely be used here instead.
  • The Domain matching strategy will not support IPv4 or IPv6 addresses, due to limitations in the psl crate. This can also likely be implemented using regular expressions in the future.

⏰ 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 (service logic only - no UI presentation)
  • 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

@CLAassistant
Copy link

CLAassistant commented Aug 30, 2025

CLA assistant check
All committers have signed the CLA.

@harr1424
Copy link
Author

harr1424 commented Sep 1, 2025

Reviewers, if you prefer I squash my commits into a clean single commit, please let me know.

@Patrick-Pimentel-Bitwarden

This is really cool work! Thank you @harr1424. As we take in community prs, we do an evaluation of priority with our project managers. I will surface the ticket (not seeing one generated at the moment), let me get back to you.

@Patrick-Pimentel-Bitwarden

Reviewers, if you prefer I squash my commits into a clean single commit, please let me know.

There is no need to squash your commits now, when merging to main we do a squash commit

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.

3 participants