Implement chunked encoding for TXT records in DNS Message Record clas…#39
Implement chunked encoding for TXT records in DNS Message Record clas…#39
Conversation
…s and add corresponding unit tests for multi-chunk handling.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThis change updates TXT record handling to support multi-chunk RDATA encoding: TXT data is split into chunks of up to 255 bytes, each prefixed with a one-byte length, and the encoder concatenates those length-prefixed chunks. Decoding concatenates multiple chunks back into the original RDATA. Unit tests were added to cover encoding and decoding round-trips for multi-chunk TXT records and to validate chunk sizes and RDLENGTH for long TXT payloads. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/DNS/Message/Record.php`:
- Around line 450-464: The TXT encoder in the TYPE_TXT branch of Record.php
builds no character-strings when $this->rdata is empty, producing RDLENGTH=0
which breaks decode(); change the logic in the TYPE_TXT case (in the encode
method handling $this->rdata, using self::MAX_TXT_CHUNK) so that if
strlen($this->rdata) === 0 you emit a single zero-length character-string
(chr(0)) instead of an empty string; leave the existing chunking behavior
unchanged for non-empty rdata.
In `@tests/unit/DNS/Message/RecordTest.php`:
- Around line 361-382: The test testEncodeTxtRecordWithMultipleChunks
incorrectly expects two 5-byte TXT chunks and an RDLENGTH of 10 for rdata
'helloworld' even though Record::encode() will emit a single chunk (length byte
+ payload) for any chunk <= 255 bytes; fix the test by updating the expected
encoding to a single TXT chunk (length byte 0x0A followed by "helloworld" and
RDLENGTH = 11) or change the test to use an rdata >255 bytes to actually
exercise multi-chunk behavior (keep references to Record::TYPE_TXT,
Record::CLASS_IN, and the Record::encode() call when updating assertions).
…or for exactly 256 bytes of rdata, ensuring correct split into two chunks and verifying round-trip encoding/decoding.
…haracter-string, ensuring compliance with RFC 1035.
There was a problem hiding this comment.
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 `@tests/unit/DNS/Message/RecordTest.php`:
- Around line 374-376: The failing CI is due to trailing whitespace on the blank
line after the $encoded = $record->encode(); statement in RecordTest
(tests/unit/DNS/Message/RecordTest.php); remove the trailing spaces from that
blank line (or delete the blank line) so the file has no whitespace-only lines,
and re-run PHP Pint (./vendor/bin/pint --config pint.json or pint --write) to
ensure the file conforms to PSR-12.
2 participants
…s and add corresponding unit tests for multi-chunk handling.
Summary by CodeRabbit
Bug Fixes
Tests