Skip to content

fix: decode NDEF URI record identifier-code byte on NFC scan#95

Merged
wizzomafizzo merged 1 commit into
mainfrom
fix/ndef-uri-record-decoding
Apr 18, 2026
Merged

fix: decode NDEF URI record identifier-code byte on NFC scan#95
wizzomafizzo merged 1 commit into
mainfrom
fix/ndef-uri-record-decoding

Conversation

@wizzomafizzo

@wizzomafizzo wizzomafizzo commented Apr 18, 2026

Copy link
Copy Markdown
Member
  • NDEF URI records (e.g. Zaparoo-branded https://zpr.au/... cards) were decoded as \u0004zpr.au/... because the URI identifier-code byte (0x04 = https://) was passed raw through int2char with no prefix expansion. This caused isZapUrl to silently return false and forwarded a malformed string to CoreAPI.run.
  • readNfcEvent in src/lib/nfc.ts now checks record.tnf and record.type, delegating to the plugin's existing NfcUtils helpers: getTextFromNdefTextRecord for Text records (also fixes variable-length language code handling) and getUriFromNdefUriRecord + mapUriIdentifierCodeToString for URI records. Non-WellKnown and unrecognised types fall back to the previous int2char behaviour.
  • Updated the global NFC automock (__mocks__/@capawesome-team/capacitor-nfc.ts) to export TypeNameFormat and RecordTypeDefinition enums and implement the full set of NfcUtils methods used by readNfcEvent.
  • Extended nfc.pure.test.ts and nfc.test.ts with URI record cases covering https://, http://www., no-prefix, unknown identifier code, empty URI body, and non-WellKnown TNF fallback.

Summary by CodeRabbit

  • Bug Fixes

    • Improved NFC tag reading with better parsing of text and URI records from NDEF records.
  • Tests

    • Added comprehensive test coverage for NDEF text record decoding with various language codes and edge cases.
    • Added test coverage for NDEF URI record decoding with multiple URI prefix identifiers.

Cards containing NDEF URI records (e.g. https://zpr.au/...) were
decoded as \u0004zpr.au/... because the URI identifier-code byte
(0x04 = https://) was passed raw through int2char with no prefix
expansion. readNfcEvent now inspects record.tnf and record.type,
delegating to NfcUtils helpers for proper Text and URI decoding.

Also fixes the text-record path to use getTextFromNdefTextRecord,
which handles variable-length language codes correctly instead of
the previous hardcoded 3-byte slice heuristic.
@coderabbitai

coderabbitai Bot commented Apr 18, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The pull request adds NFC NDEF record parsing utilities to the capacitor-nfc mock, including enums for record type formats and helper methods to decode text and URI records. The implementation is updated to use these utilities instead of manual payload parsing, with corresponding test updates.

Changes

Cohort / File(s) Summary
NFC Mock Utilities
__mocks__/@capawesome-team/capacitor-nfc.ts
Added TypeNameFormat and RecordTypeDefinition enums, plus five new NfcUtils helper methods to map record type bytes, decode NDEF text/URI payloads, and resolve URI prefix identifier codes.
Unit Tests
src/__tests__/unit/lib/nfc.pure.test.ts, src/__tests__/unit/lib/nfc.test.ts
Updated mock imports for async/actual module preservation, extended NDEF text record tests to cover language codes and short payloads, added comprehensive NDEF URI record tests with various prefix identifier codes, and verified fallback behavior for unknown record types.
NFC Implementation
src/lib/nfc.ts
Refactored readNfcEvent to use NfcUtils helpers for parsing NDEF tnf/type fields instead of manual payload slicing; handles WellKnown text and URI records with proper prefix resolution and fallback to int2char conversion.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 NDEF records once obscured in bytes,
Now hop through parsing utilities light—
Text decodes, URIs bloom so bright,
Type-safe parsing sets things right! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main fix: decoding NDEF URI record identifier-code bytes on NFC scan, which is the primary purpose of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ndef-uri-record-decoding

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Apr 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/lib/nfc.ts 94.44% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@__mocks__/`@capawesome-team/capacitor-nfc.ts:
- Around line 123-129: The mock getTextFromNdefTextRecord reads the entire
status byte as langLen which is wrong for NDEF text records; update it to mask
the status byte with 0x3F to obtain the language length (langLen = payload[0] &
0x3F) and inspect the encoding bit (payload[0] & 0x80) to choose the correct
TextDecoder ("utf-16" when set, otherwise "utf-8"); then decode from
payload.slice(langLen + 1) (or new Uint8Array(payload).subarray(langLen + 1))
and return that text so the mock matches real NDEF behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7c75e50d-9770-45d4-8276-672ff2421a78

📥 Commits

Reviewing files that changed from the base of the PR and between 7e364ae and 399c11c.

📒 Files selected for processing (4)
  • __mocks__/@capawesome-team/capacitor-nfc.ts
  • src/__tests__/unit/lib/nfc.pure.test.ts
  • src/__tests__/unit/lib/nfc.test.ts
  • src/lib/nfc.ts

Comment thread __mocks__/@capawesome-team/capacitor-nfc.ts
@wizzomafizzo
wizzomafizzo merged commit 9f90693 into main Apr 18, 2026
6 checks passed
@wizzomafizzo
wizzomafizzo deleted the fix/ndef-uri-record-decoding branch April 18, 2026 03:58
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.

1 participant