Skip to content

fix: comprehensive NDEF parsing - replace ParseNDEFData with ParseNDEFMessage for all tag types#5

Merged
wizzomafizzo merged 4 commits intomainfrom
comprehensive-ndef-fix
Aug 14, 2025
Merged

fix: comprehensive NDEF parsing - replace ParseNDEFData with ParseNDEFMessage for all tag types#5
wizzomafizzo merged 4 commits intomainfrom
comprehensive-ndef-fix

Conversation

@wizzomafizzo
Copy link
Member

Summary

This PR provides a comprehensive fix for NDEF parsing across all NFC tag types by replacing the broken ParseNDEFData function with the proper ParseNDEFMessage implementation.

Problem Description

The current codebase had a fundamental architectural issue where ParseNDEFData only parsed text records, causing all other NDEF record types to fail with "no NDEF data" errors:

  • URI records failed (e.g., website links, mailto links)
  • WiFi credential records failed
  • vCard contact records failed
  • Smart poster records failed
  • Any non-text NDEF content failed

This affected all tag types: MIFARE Classic, NTAG, and FeliCa.

Root Cause Analysis

  • ParseNDEFData was a legacy function with hardcoded text-only parsing
  • It had a TODO comment: // TODO: Properly parse full NDEF structure
  • ParseNDEFMessage is the modern implementation using go-ndef library
  • ParseNDEFMessage properly handles all NDEF record types

Changes Made

Code Updates

  • mifare.go:450 - Fixed MIFARE Classic ReadNDEF
  • ntag.go:227 - Fixed NTAG ReadNDEF (first method)
  • ntag.go:395 - Fixed NTAG readNDEFBlockByBlock method
  • felica.go:362 - Fixed FeliCa ReadNDEF
  • parameter_validation_test.go - Updated test references
  • ndef_construction_test.go - Updated test function name and references

Code Removal

  • ndef.go - Removed the broken ParseNDEFData function (67 lines of dead code)

Testing

  • ✅ All existing tests pass
  • ✅ No breaking changes to public API
  • ✅ Maintains backward compatibility for text records
  • ✅ Enables support for URI, WiFi, vCard, and other NDEF record types

Impact

This fix enables proper NDEF parsing for modern NFC applications that use:

  • QR code replacement with URI records
  • WiFi credential sharing
  • Contact information exchange
  • Smart poster advertisements
  • Mixed-content NDEF messages

Relationship to PR #2

This extends and supersedes the fix originally proposed in PR #2, providing a complete solution that affects all tag types rather than just MIFARE Classic, plus removes the broken legacy code entirely.

kamikazechaser and others added 4 commits August 13, 2025 12:26
…ds present

At the moment it attempts to parse the first text only record which will fail (no NDEF data) if the tag contains other kinds of records e.g. a URI exclusively
- Fix MIFARE Classic, NTAG, and FeliCa tags to use ParseNDEFMessage
- Update all related tests to use the correct function
- This ensures URI, WiFi, vCard, and other non-text NDEF records work properly
- ParseNDEFData only handled text records, causing 'no NDEF data' errors for other types

Fixes comprehensive NDEF parsing for all tag types, not just MIFARE Classic.
- Function was fundamentally broken (text-only parsing)
- All usages have been replaced with ParseNDEFMessage
- Eliminates 67 lines of dead/broken code
- Prevents future accidental usage of the broken function
- Remove extra blank line in ndef.go (gci formatting)
- Break long line in ndef_construction_test.go (line length limit)

All linters now pass.
@wizzomafizzo wizzomafizzo merged commit fcc3f79 into main Aug 14, 2025
8 checks passed
@wizzomafizzo wizzomafizzo deleted the comprehensive-ndef-fix branch August 14, 2025 01:30
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