On Thu, Aug 13, 2026 at 3:25 PM arpitjain099 (via GitHub) <[email protected]> wrote: > > > arpitjain099 commented on PR #407: > URL: https://github.com/apache/commons-net/pull/407#issuecomment-5285392846 > > Tested. Built master and the PR branch side by side and ran the same > inputs through each. > > The fix is correct. On the PR branch all three variants of the > unterminated option value return TFTPPacketException, where master gives > ArrayIndexOutOfBoundsException, "OK {blksize=1024}" and TFTPPacketException > depending on what sits past getLength(). Well formed packets are unaffected: > a terminated option still parses to {blksize=1024} with or without spare > capacity, and a request with no options still parses to {}. > > One behaviour change worth deciding before merge. A packet whose last > option has a key and its NUL but no value field is now accepted with an empty > value: > > key + NUL, then nothing master: ArrayIndexOutOfBoundsException PR: > OK {blksize=} > key + NUL, spare stale master: TFTPPacketException PR: > OK {blksize=} > > After the key loop stops on the NUL, index is incremented to dataLen, so > the value loop's guard is false immediately and the value reads as empty. RFC > 2347 terminates both name and value, so I would expect "Invalid option > format" here. Master was not right either, so this is a choice about what > correct looks like rather than a regression. The two tests on the branch do > not cover it. > > Minor: the new dataLen < 1 guard is followed by a read of data[1], which > wants dataLen < 2. Unreachable through newTFTPPacket, which rejects lengths 0 > through 2 first, so it is only about the constructor being self consistent.
Yep, good one. I've removed the check which was both incorrect and redundant, squahed and pushed. Gary > > > -- > This is an automated message from the Apache Git Service. > To respond to the message, please log on to GitHub and use the > URL above to go to the specific comment. > > To unsubscribe, e-mail: [email protected] > > For queries about this service, please contact Infrastructure at: > [email protected] >
