chore: merge pn532 upstream changes#61
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🧰 Additional context used📓 Path-based instructions (1)**/*⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📓 Common learnings🔇 Additional comments (1)
WalkthroughRemoved PN532 write-completion trigger and replaced trigger handling with callback-based automations; switched PN532 read-timing from a sentinel uint32_t to an optional; adjusted MIFARE Ultralight NDEF indexing and extended-length encoding layout; tightened PN532 SPI response validation and added disable-on-failure paths. Changes
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 718ac64b-faad-43f1-b0ee-d2a1c818fc67
📒 Files selected for processing (1)
components/pn532_spi/pn532_spi.cpp
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
assess the relevance of the changes to the purpose of the PR and do not mention anything if they are relevant.
Files:
components/pn532_spi/pn532_spi.cpp
🔇 Additional comments (1)
components/pn532_spi/pn532_spi.cpp (1)
91-95: Correct preamble validation and cleanup.The
||check now rejects any malformed start sequence, and the addeddisable()calls keep the SPI transaction balanced on both early-return paths.Also applies to: 108-110
| // full length of message, including command response (minimum 2: TFI + command response) | ||
| uint16_t full_len = header[3]; | ||
| if (full_len < 2) { | ||
| ESP_LOGV(TAG, "read data has no payload"); | ||
| this->disable(); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Validate the decoded extended-frame length before using it.
full_len is checked before the extended-frame branch overwrites it. If a malformed extended frame decodes to 0 or 1, Line 134 still executes with an invalid length, which can turn into a 65535-byte read or an invalid data[len - 1] access. Move the < 2 guard after the extended-length decode so both frame formats are covered.
🐛 Proposed fix
- // full length of message, including command response (minimum 2: TFI + command response)
- uint16_t full_len = header[3];
- if (full_len < 2) {
- ESP_LOGV(TAG, "read data has no payload");
- this->disable();
- return false;
- }
+ // full length of message, including command response (minimum 2: TFI + command response)
+ uint16_t full_len = header[3];
if (extended_frame) {
ESP_LOGV(TAG, "Abnormal length and checksum, possible Extended Frame");
header.resize(10);
this->read_array(header.data() + 7, 3);
ESP_LOGV(TAG, "EF: Header data: %s", format_hex_pretty(header).c_str());
if ((uint8_t)(header[5] + header[6] + header[7]) != 0) {
ESP_LOGV(TAG, "EF: read data invalid header!");
this->disable();
return false;
}
full_len = ((((uint16_t)header[5]) << 8) | header[6]);
}
+ if (full_len < 2) {
+ ESP_LOGV(TAG, "read data has no payload");
+ this->disable();
+ return false;
+ }Also applies to: 121-132
rednblkx
force-pushed
the
upstream/pn532
branch
from
84c0d46 to
ddbc8ba
Compare
1 participant
Summary by CodeRabbit
Refactor
Bug Fixes