Skip to content

Conversation

@berZKerk
Copy link

@berZKerk berZKerk commented Aug 16, 2025

Added input validation to prevent potential runtime errors when processing malformed date strings.

Summary by CodeRabbit

  • Bug Fixes
    • Improved date parsing: accepts short YYMMDD inputs, preserves handling of full timestamps, and rejects empty or malformed dates with clear errors.
    • Prevents silent failures in features depending on dates; ensures more consistent behavior across platforms.
    • No visible UI changes; users may see explicit error messages for invalid dates.

remicolin and others added 2 commits June 25, 2025 20:06
* remove sdk/tests (selfxyz#622)

* remove sdk/tests

* chore: update yarn.lock

---------

Co-authored-by: Ayman <[email protected]>

* fix: add range check on paddedInLength of shaBytesDynamic (selfxyz#623)

* fix ci (selfxyz#626)

* implement self uups upgradeable (selfxyz#592)

* implement self uups upgradeable

* small changes in identityVerificationHubImplV2

* delete aderyn.toml

* chore: add custom verifier

* chnage return output

* feat: use self structs and a Generic output struct

* feat: add userIdentifier, nullifier, forbiddencountries to returned output

* add root view functions from registry

* fix: build and compilation errors

* add userDefined data into selfVerificationRoot

* "resolve conflicts"

* fix compilation problem

* fix how to register verification config

* test: CustomVerifier

* fix verification root and hub integration

* add scope check in hub impl

* replace poseidon hash to ripemd+sha256

* add todo list

* feat: refactor and add test cases for generic formatter

* add performUserIdentifierCheck in basicVerification

* change how to handle additionalData and fix stack too deep

* start adding test codes

* fix dependency problems in monorepo

* fix: forbidden countries (selfxyz#612)

LGTM!

* able to run test code

* pass happy path

* delete unused codes

* change error code name, add caller address validation and add scripts to run test and build in monorepo

* add all test cases in vcAndDisclose flow

* remove comment out

* chore: use actual user identifier outputs

* success in registration tests

* cover all cases

* pass contractVersion instead of circuitVersion

* fix disclose test

* chore: add natspecs for ImplHubV2, CustomVerifier and GenericFormatter

* change val name and remove unused lines

* add val name change

* remove userIdentifier from return data

* feat: use GenericDiscloseOutput struct in verfication hook  fix test cases for user identifier

* chore: change the function order for Hub Impl V2 (selfxyz#625)

* fix nat specs

* add nat spec in SelfStructs

---------

Co-authored-by: Ayman <[email protected]>
Co-authored-by: Nesopie <[email protected]>

* prettier (selfxyz#629)

* CAN auth - android (selfxyz#613)

* add missed files

* add NFCMethodSelectionScreen

* bump android build

---------

Co-authored-by: Justin Hernandez <[email protected]>

* feat: add MRZ correction method to NFCMethodSelectionScreen (selfxyz#627)

* add npm auth token env (selfxyz#632)

* bump sdk version (selfxyz#633)

* publish npm package when merging on dev

* bump common sdk version

* replace yarn publish by npm publish

* update common package version

* Simplify dev mode gesture (selfxyz#635)

* Simplify developer mode gesture

* Enable dev mode on MockData screen with five taps

* add build smt function to common sdk

* update vc_and_disclose_id test (dev branch) (selfxyz#641)

* fix: vc_and_disclose_id test

* chore: yarn prettier

* Show modal on NFC scan error (selfxyz#642)

* Add help button and error modal actions

* fix the screen management

* yarn nice

* Bump build v2.5.4: ios 132; android 71 (selfxyz#631)

* bump version and build numbers

* remove tamagui/toast

* fix marketing version

* fix: update TD1 and TD3 checks (selfxyz#643)

* bum yarn.lock

* Bump build: ios 133; android 72 and build fixes (selfxyz#654)

* update gesture version and bump android build

* bump and fix ios build

* update lock files

* fixes

* fix fotoapparat library source

* Update example contracts to include EUID usage (selfxyz#656)

* refactor: update HappyBirthday contract to V2 with support for E-Passport and EUID cards, introduce bonus multipliers, and enhance verification logic

* refactor: update Airdrop contract to V2 with support for E-Passport and EU ID Card attestations

* refactor: remove BASIS_POINTS constant from Airdrop contract

* feat: introduce SelfIdentityERC721 contract for issuing NFTs based on verified identity credentials, replacing SelfPassportERC721

* fix: update verification functions in Airdrop, HappyBirthday, and SelfIdentityERC721 contracts to use customVerificationHook

* cherry pick commit from add-test-self-verification...

* block non-dev pr to main branch

* audit fixes (selfxyz#645)

* merge dev branch into main (selfxyz#624)

* remove sdk/tests (selfxyz#622)

* remove sdk/tests

* chore: update yarn.lock

---------

Co-authored-by: Ayman <[email protected]>

* fix: add range check on paddedInLength of shaBytesDynamic (selfxyz#623)

* fix ci (selfxyz#626)

---------

Co-authored-by: Ayman <[email protected]>
Co-authored-by: Vishalkulkarni45 <[email protected]>

* update contracts (selfxyz#628)

* remove sdk/tests (selfxyz#622)

* remove sdk/tests

* chore: update yarn.lock

---------

Co-authored-by: Ayman <[email protected]>

* fix: add range check on paddedInLength of shaBytesDynamic (selfxyz#623)

* fix ci (selfxyz#626)

* implement self uups upgradeable (selfxyz#592)

* implement self uups upgradeable

* small changes in identityVerificationHubImplV2

* delete aderyn.toml

* chore: add custom verifier

* chnage return output

* feat: use self structs and a Generic output struct

* feat: add userIdentifier, nullifier, forbiddencountries to returned output

* add root view functions from registry

* fix: build and compilation errors

* add userDefined data into selfVerificationRoot

* "resolve conflicts"

* fix compilation problem

* fix how to register verification config

* test: CustomVerifier

* fix verification root and hub integration

* add scope check in hub impl

* replace poseidon hash to ripemd+sha256

* add todo list

* feat: refactor and add test cases for generic formatter

* add performUserIdentifierCheck in basicVerification

* change how to handle additionalData and fix stack too deep

* start adding test codes

* fix dependency problems in monorepo

* fix: forbidden countries (selfxyz#612)

LGTM!

* able to run test code

* pass happy path

* delete unused codes

* change error code name, add caller address validation and add scripts to run test and build in monorepo

* add all test cases in vcAndDisclose flow

* remove comment out

* chore: use actual user identifier outputs

* success in registration tests

* cover all cases

* pass contractVersion instead of circuitVersion

* fix disclose test

* chore: add natspecs for ImplHubV2, CustomVerifier and GenericFormatter

* change val name and remove unused lines

* add val name change

* remove userIdentifier from return data

* feat: use GenericDiscloseOutput struct in verfication hook  fix test cases for user identifier

* chore: change the function order for Hub Impl V2 (selfxyz#625)

* fix nat specs

* add nat spec in SelfStructs

---------

Co-authored-by: Ayman <[email protected]>
Co-authored-by: Nesopie <[email protected]>

* prettier (selfxyz#629)

---------

Co-authored-by: Ayman <[email protected]>
Co-authored-by: Vishalkulkarni45 <[email protected]>
Co-authored-by: nicoshark <[email protected]>
Co-authored-by: Nesopie <[email protected]>

* fix: vc_and_disclose_id test (selfxyz#640)

* fix: vc_and_disclose_id test

* chore: yarn prettier

* fix: check if a config id exists

* chore: change the function where the config not set verification is happening

* fix: add await

* feat: add getConfigId function in SelfVerificationRoot (selfxyz#650)

* feat: add getConfigId function in SelfVerificationRoot

* update comment

---------

Co-authored-by: motemotech <[email protected]>

* chore: fix ofac end index in eu id cards

* chore: fix tests

* fix: example contracts and tests

---------

Co-authored-by: turnoffthiscomputer <[email protected]>
Co-authored-by: Vishalkulkarni45 <[email protected]>
Co-authored-by: nicoshark <[email protected]>

* Update deployment module for Identity Verification Hub V2 with detailed documentation and library linkage for CustomVerifier. Update initialization process to reflect changes in V2 implementation, ensuring proper setup for proxy deployment. (selfxyz#658)

* publish npm-package (selfxyz#651)

* App/eu id updates (selfxyz#638)

* fix build issues

* generate disclosure proof with euids

* generate disclosure proof with euids

* Eu id updates 2 (selfxyz#648)

* update vc_and_disclose_id test (dev branch) (selfxyz#641)

* fix: vc_and_disclose_id test

* chore: yarn prettier

* Show modal on NFC scan error (selfxyz#642)

* Add help button and error modal actions

* fix the screen management

* yarn nice

* Bump build v2.5.4: ios 132; android 71 (selfxyz#631)

* bump version and build numbers

* remove tamagui/toast

* fix marketing version

* fix: update TD1 and TD3 checks (selfxyz#643)

* bum yarn.lock

* add version and user defined data

---------

Co-authored-by: Vishalkulkarni45 <[email protected]>
Co-authored-by: Justin Hernandez <[email protected]>
Co-authored-by: Seshanth.S🐺 <[email protected]>

* remove the mock user define data

* get the useridentifier as a hash from the user defined data

* chore: add version and userDefinedData

* feat: use the version in register / dsc proofs as well

* update calculateUserIdentifierHash

* yarn nice

* refactor: consolidate user context data handling and update payload structure

* fix typing issues on sha1

* remove console.log(sha1)

* fix sha1 import

* refactor: streamline userDefinedData handling and adjust payload type for circuit

* refactor: update sha1 usage and enhance logging in calculateUserIdentifierHash

* yarn nice

* yarn lint common

* use ts-ignore for sha1 import

* fix app ci tests

* fix typing issue

* remove unused ts-ignore

* cast uuid before calling generateinputs

* bump qrcode version

* add tsup on the qrcode sdk

* fix: exports on selfxyz/qrcode

* update how we define config.version

* fix yarn imports

* yarn format

---------

Co-authored-by: Vishalkulkarni45 <[email protected]>
Co-authored-by: Justin Hernandez <[email protected]>
Co-authored-by: Seshanth.S🐺 <[email protected]>
Co-authored-by: Ayman <[email protected]>

* Hotfix contract compile error (selfxyz#660)

* Fix previous rebase error

* Refactor deployment module for Identity Verification Hub V2.

* Fix/sdk (selfxyz#652)

* fix: sdk build configs

* chore: SelfBackendVerifier (WIP)

* feat: add custom verification

* feat: consider destination chain in user defined data

* chore: export attestation id

* chore: export attestation id

* chore: export config storage

* chore: don't throw an error if the proof is not valid

* chore: trim abi and rm typechain types

* refactor

* chore: rm unnecessary exports

* 📝 Add docstrings to `fix/sdk` (selfxyz#653)

Docstrings generation was requested by @remicolin.

* selfxyz#652 (comment)

The following files were modified:

* `sdk/core/src/utils/hash.ts`
* `sdk/core/src/utils/proof.ts`
* `sdk/core/src/utils/utils.ts`

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* review fixes

* chore: fix package.json cjs types

* chore: add minor changes to checks

* feat: add InMemoryConfigStore, allIds constant and verificationResult type

* chore: export Verification config

* feat: change the verification config types

* fix: throw issues early if verification config is null

* fix: update yarn.lock file

* chore: lint

* fix: rm ts expect error directive

* fix: contract tests

* use excluded countries instead forbidden countries list

* chore: change types in constnats

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Update npm-publish workflow and bump core package version to 1.0.0 (selfxyz#661)

* update import

* Update get verification config visibility (selfxyz#664)

* Update deployment module for Identity Verification Hub V2 to correct file paths and module name for deployment commands.

* Add troubleshooting documentation for verification issues in deployHubV2.ts. Include manual verification steps and common failure reasons to assist users during deployment.

* Change visibility of getVerificationConfigV2 function from internal to public in IdentityVerificationHubImplV2 contract to allow external access.

* Apply BUSL v1.1 license headers to app (selfxyz#665)

* Add BSL license headers to app sources

* prettier

* fix license reference - https://spdx.org/licenses/BUSL-1.1.html

* bump build: android 73 (selfxyz#659)

* Contracts/deploy staging (selfxyz#668)

* update scripts

* deploy vc and disclose id

* fix the deployment scripts on staging

* update yarn.lock

* bump ios build and version (selfxyz#669)

* configure coderabbitai (selfxyz#670)

* tweak coderabbit

* bump

* more thorough test spec

* Apply BSL to app codebase (selfxyz#639)

* Clean up root license wording

* Simplify SPDX header

* simplify license and rename BSL to BUSL

* fix merge issues

* fix missing method

---------

Co-authored-by: Justin Hernandez <[email protected]>

* SEL-423 apply xcode build suggestions (selfxyz#671)

* apply recommended app settings from xcode

* stick to portrait orientation and update target settings

* remove app clip references

* Circuit audit fixes  (selfxyz#644)

* feat: add range checks before use of LessEqThan and SelectSubArray

* fix: Num2Bits_strict to constrain virtualKey

* bump core version

* bump core version and fix ci

* chore: use npm_auth_token in yarnrc

* chroe: rm yarnrc changes

* chore: update npm publish

* chore: run npm publish manually

* chore: change hub contract address (selfxyz#675)

* Update npm-publish.yml

* chore: use proper secret when publishing

* feat: enable publishing if workflow was triggered manually

* Contracts/update verifier (selfxyz#673)

* update hardhat config

* update vc and disclose verifier

* update vc and disclose verifier script and run it

* update test self verification root

* update verifier

* bump sdk version and use new hub address

* chore: update zk-kit binary merkle root dep (selfxyz#674)

* refactor deployment scripts (selfxyz#678)

* feat: add register eu id instances (selfxyz#682)

* feat: add register eu id instances

* feat: add new instances

* chore: update scripts

* chore: fix sig alg

* chore: rm circuits

---------

Co-authored-by: Ayman <[email protected]>
Co-authored-by: Vishalkulkarni45 <[email protected]>
Co-authored-by: nicoshark <[email protected]>
Co-authored-by: Nesopie <[email protected]>
Co-authored-by: Seshanth.S🐺 <[email protected]>
Co-authored-by: Justin Hernandez <[email protected]>
Co-authored-by: Justin Hernandez <[email protected]>
Co-authored-by: Kevin Lin <[email protected]>
Co-authored-by: kevinsslin <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Eric Nakagawa <[email protected]>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 16, 2025

Walkthrough

formatDateToYYMMDD in app/src/utils/utils.ts gained stricter input handling: it now throws on empty input, returns 6-character YYMMDD strings unchanged, extracts YYMMDD from 10+ character YYYY-MM-DD-like strings, and throws for other invalid lengths. TD1_REGEX was converted from named capture groups to a non-named regex (test usage unchanged). No exported signatures changed.

Changes

Cohort / File(s) Summary
Date parsing & regex tweaks
app/src/utils/utils.ts
Reworked formatDateToYYMMDD input validation: throws Error for empty input, returns 6-char input directly (YYMMDD), extracts YYMMDD from 10+ char ISO-like strings, and throws on other invalid lengths. Replaced named capturing groups in TD1_REGEX with a non-named capturing regex; no change to exports/signatures.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant Utils as utils.formatDateToYYMMDD

  Caller->>Utils: formatDateToYYMMDD(inputDate)
  alt inputDate is empty
    Utils-->>Caller: throw Error("Invalid date format: input string cannot be empty")
  else if inputDate length == 6
    Utils-->>Caller: return inputDate (YYMMDD)
  else if inputDate length >= 10
    Utils->>Utils: extract substrings (year=2..4, month=5..7, day=8..10)
    Utils-->>Caller: return year+month+day
  else
    Utils-->>Caller: throw Error("Invalid date format: expected YYMMDD (6 chars) or YYYY-MM-DD format (10+ chars)")
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A tiny date, six chars bright,
Another spans the ISO night.
Empty inputs get a firm “nope,”
Substrings stitch the needed scope.
Regex trimmed — the code stays light.

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@giga-agent
Copy link

giga-agent bot commented Aug 16, 2025

Giga Summary

• Added input validation to formatDateToYYMMDD function to prevent substring errors
• Impact: Enhances robustness by preventing runtime errors from malformed date strings
• Changes:

  • Added validation to check if inputDate is not null and has a length of at least 10 characters
  • Throws an error if the input date is invalid
    • Focus:
  • Ensure the validation logic correctly handles edge cases and potential malformed inputs

Quality Assessment

• Key strengths: Effective input validation, focused fix.
• Critical issues: None identified.
• Recommendation: Approve.

Quality Score: 9/10 (Threshold: 7/10)

💬 Detailed comments have been added to specific code changes.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🔭 Outside diff range comments (1)
app/src/utils/utils.ts (1)

49-63: Handle potential exceptions from formatDateToYYMMDD in PassportCameraScreen

We’ve confirmed that formatDateToYYMMDD now throws on inputs shorter than 10 characters. On iOS branches (lines 72 & 74), those calls aren’t wrapped, so an unexpected or malformed date string will bubble up and crash the app. Please ensure you either pre-validate or catch errors around these calls.

• File: app/src/screens/passport/PassportCameraScreen.tsx
– Lines 72 and 74: wrap formatDateToYYMMDD(dateOfBirth) and formatDateToYYMMDD(dateOfExpiry) in a try/catch (or validate dateOfBirth/dateOfExpiry length ≥ 10 first).
– Fall back to a safe default or show a user‐friendly error message rather than letting an uncaught exception crash the app.

Suggested diff snippet:

--- a/app/src/screens/passport/PassportCameraScreen.tsx
+++ b/app/src/screens/passport/PassportCameraScreen.tsx
@@ -69,7 +69,16 @@ export function PassportCameraScreen() {
-      const formattedDateOfBirth =
-        Platform.OS === 'ios' ? formatDateToYYMMDD(dateOfBirth) : dateOfBirth;
-      const formattedDateOfExpiry =
-        Platform.OS === 'ios' ? formatDateToYYMMDD(dateOfExpiry) : dateOfExpiry;
+      let formattedDateOfBirth = dateOfBirth;
+      let formattedDateOfExpiry = dateOfExpiry;
+      if (Platform.OS === 'ios') {
+        try {
+          if (dateOfBirth.length >= 10) {
+            formattedDateOfBirth = formatDateToYYMMDD(dateOfBirth);
+          }
+          if (dateOfExpiry.length >= 10) {
+            formattedDateOfExpiry = formatDateToYYMMDD(dateOfExpiry);
+          }
+        } catch (err) {
+          // TODO: surface validation error to user instead of crashing
+          console.error('Invalid passport date format', err);
+        }
+      }
🧹 Nitpick comments (3)
app/src/utils/utils.ts (3)

51-60: Strengthen validation: enforce ISO-like prefix and trim before slicing

Length alone won’t catch cases like "2025/08/16…" or leading/trailing spaces. Consider trimming and validating the first 10 chars match YYYY-MM-DD. This keeps the function fast and reduces silent bad outputs.

Proposed patch:

 export function formatDateToYYMMDD(inputDate: string) {
-  // Validate input string length to prevent substring errors
-  if (!inputDate || inputDate.length < 10) {
-    throw new Error('Invalid date format: input string must be at least 10 characters long');
-  }
+  // Normalize and validate the ISO-like date prefix (YYYY-MM-DD)
+  if (!inputDate) {
+    throw new TypeError('Invalid date format: expected a non-empty string');
+  }
+  const s = inputDate.trim();
+  if (s.length < 10 || !/^\d{4}-\d{2}-\d{2}/.test(s)) {
+    throw new TypeError('Invalid date format: expected YYYY-MM-DD at the start of the string');
+  }
 
   // Extract the date components directly from the input string
-  const year = inputDate.substring(2, 4); // Get YY part
-  const month = inputDate.substring(5, 7); // Get MM part
-  const day = inputDate.substring(8, 10); // Get DD part
+  const year = s.substring(2, 4); // Get YY part
+  const month = s.substring(5, 7); // Get MM part
+  const day = s.substring(8, 10); // Get DD part

51-55: Prefer TypeError for invalid argument shape

Minor: throwing TypeError is more semantically accurate for invalid argument values than a generic Error.

If you keep the current approach:

-    throw new Error('Invalid date format: input string must be at least 10 characters long');
+    throw new TypeError('Invalid date format: input string must be at least 10 characters long');

49-63: Add unit tests for valid and invalid inputs

Recommend adding tests covering:

  • Valid: "2025-08-16 00:00:00 +0000" → "250816"
  • Valid: "2025-08-16" → "250816"
  • Invalid: empty string, whitespace only, shorter than 10 chars, wrong separators like "2025/08/16", non-digits in date

I can draft a Jest test file with these cases if helpful.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between cb2ef91 and 93e8d5f.

📒 Files selected for processing (1)
  • app/src/utils/utils.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
app/src/**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit Configuration File

app/src/**/*.{ts,tsx,js,jsx}: Review React Native TypeScript code for:

  • Component architecture and reusability
  • State management patterns
  • Performance optimizations
  • TypeScript type safety
  • React hooks usage and dependencies
  • Navigation patterns

Files:

  • app/src/utils/utils.ts
🔇 Additional comments (1)
app/src/utils/utils.ts (1)

51-55: Good defensive guard to prevent malformed inputs

The upfront length check is a pragmatic way to stop bad inputs before slicing. Low overhead, improves robustness, and avoids downstream surprises.

@transphorm
Copy link
Member

@berZKerk thanks for opening this pull request. can you please re-create and open against the dev branch? thank you!

// Function to format date from 'YYYY-MM-DD 00:00:00 +0000' to 'YYMMDD'
export function formatDateToYYMMDD(inputDate: string) {
// Validate input string length to prevent substring errors
if (!inputDate || inputDate.length < 10) {
Copy link
Contributor

Choose a reason for hiding this comment

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

couldnt it be equal to 10 too?

@berZKerk berZKerk requested a review from aaronmgdr August 26, 2025 11:11
@berZKerk
Copy link
Author

So basically I've made some changes related to @aaronmgdr comment, also could you please tell me how to solve all those conflicts?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/src/utils/utils.ts (1)

8-13: TD1 detection/extraction is inconsistent: regex expects line1+line2 concatenated, but you only test/slice line 0

The TD1 pattern matches ~55 chars from the concatenation of TD1 line 1 and line 2, yet isTD1 tests mrzLines[0] only, and the TD1 extraction later slices indices (30..44) that only make sense on the concatenated string. This will misclassify valid TD1 MRZs and extract incorrect DOB/expiry values.

Apply this minimal fix to test the concatenated lines and to extract from the same concatenation:

-  const isTD1 = TD1_REGEX.test(mrzLines[0]) || mrzLines[0].startsWith('I');
+  const td1Candidate = (mrzLines[0] ?? '') + (mrzLines[1] ?? '');
+  const isTD1 = TD1_REGEX.test(td1Candidate);

@@
-    const line = mrzLines[0];
+    const line = td1Candidate;

Optionally, consider also validating expected lengths before parsing (TD1 lines are 30 chars each) and removing the permissive startsWith('I') fallback to avoid false positives. I can provide a hardened parser if helpful.

Also applies to: 23-37

♻️ Duplicate comments (1)
app/src/utils/utils.ts (1)

61-70: Prefer pattern-based parsing over length checks; also support YYYYMMDD (8 digits)

Using length >= 10 can accept malformed strings (e.g., wrong separators). Match the expected prefix YYYY-MM-DD, and optionally accept YYYYMMDD which is common. This keeps the function robust while staying aligned with the PR goal (avoid substring errors).

-  // If the date is in YYYY-MM-DD format (at least 10 characters), convert it
-  if (inputDate.length >= 10) {
-    // Extract the date components directly from the input string
-    const year = inputDate.substring(2, 4); // Get YY part
-    const month = inputDate.substring(5, 7); // Get MM part
-    const day = inputDate.substring(8, 10); // Get DD part
-
-    // Concatenate components into YYMMDD format
-    return year + month + day;
-  }
+  // YYYY-MM-DD[...] (Android)
+  const mDash = inputDate.match(/^(\d{4})-(\d{2})-(\d{2})/);
+  if (mDash) {
+    const [, yyyy, mm, dd] = mDash;
+    return yyyy.slice(2) + mm + dd;
+  }
+
+  // YYYYMMDD (fallback)
+  if (/^\d{8}$/.test(inputDate)) {
+    return inputDate.slice(2);
+  }

Note: The earlier reviewer’s question “couldn’t it be equal to 10 too?” is effectively addressed by matching exactly YYYY-MM-DD (10 chars prefix) instead of relying on length.

🧹 Nitpick comments (4)
app/src/utils/utils.ts (4)

52-54: Trim input before validation to avoid benign whitespace rejections

Leading/trailing spaces from OCR/platform APIs can trigger the "empty input" error. Trim first, then validate.

-export function formatDateToYYMMDD(inputDate: string) {
-  if (!inputDate) {
+export function formatDateToYYMMDD(inputDate: string) {
+  inputDate = inputDate.trim();
+  if (!inputDate) {
     throw new Error('Invalid date format: input string cannot be empty');
   }

56-60: Validate YYMMDD to be digits only

Right now any 6-char string passes. Enforce digit-only to prevent silent corruption.

-  if (inputDate.length === 6) {
-    return inputDate;
-  }
+  if (inputDate.length === 6) {
+    if (!/^\d{6}$/.test(inputDate)) {
+      throw new Error('Invalid date format: YYMMDD must be 6 digits');
+    }
+    return inputDate;
+  }

51-51: Add explicit return type and brief TSDoc for exported utility

Small clarity boost and safer refactors later.

-export function formatDateToYYMMDD(inputDate: string) {
+/**
+ * Normalizes dates to YYMMDD (UTC) from inputs like 'YYMMDD', 'YYYY-MM-DD', 'YYYYMMDD'.
+ * Throws on unsupported formats.
+ */
+export function formatDateToYYMMDD(inputDate: string): string {

49-73: Consolidate date formatting: route MainScreen through this utility to avoid duplication

MainScreen.tsx (lines 101-103 in the provided snippet) manually constructs YYMMDD via toISOString().slice(...). Consider using formatDateToYYMMDD there for a single source of truth and consistent validation behavior across platforms.

I can prepare a small PR diff for MainScreen.tsx to swap in this utility.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 93e8d5f and 6b59e01.

📒 Files selected for processing (1)
  • app/src/utils/utils.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/technical-specification.mdc)

**/*.{ts,tsx}: Define IdentityCommitment with fields: commitment (Poseidon hash), nullifier (domain-separated), timestamp (UTC number), version (circuit version), documentType ('passport' | 'eu_id_card')
Define DSCKeyCommitment with fields: publicKeyHash (Poseidon hash), certificateChain (hashes), revocationStatus (boolean), issuer (country code)
Define VerificationConfig with fields: circuitVersion (semver), complianceRules array, timeWindow (seconds, 24h), clockDrift (±5 min), trustAnchors, revocationRoots, timeSource (NTP), nullifierScope (domain separation)

Files:

  • app/src/utils/utils.ts
app/src/**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

app/src/**/*.{ts,tsx,js,jsx}: Review React Native TypeScript code for:

  • Component architecture and reusability
  • State management patterns
  • Performance optimizations
  • TypeScript type safety
  • React hooks usage and dependencies
  • Navigation patterns

Files:

  • app/src/utils/utils.ts
🧬 Code graph analysis (1)
app/src/utils/utils.ts (2)
app/src/screens/MainScreen.tsx (1)
  • date (102-104)
common/src/utils/utils.ts (1)
  • getCurrentDateYYMMDD (234-246)
🔇 Additional comments (1)
app/src/utils/utils.ts (1)

9-9: Regex change itself is fine given current usage

Switching to non-named capture groups in TD1_REGEX is OK since the code uses .test(...) only. Once the TD1 concatenation issue (above) is fixed, this regex will serve the detection purpose as intended.

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.

4 participants