Skip to content

tftp: stricter option name checks#21560

Closed
bagder wants to merge 1 commit into
masterfrom
bagder/tftp-prefix
Closed

tftp: stricter option name checks#21560
bagder wants to merge 1 commit into
masterfrom
bagder/tftp-prefix

Conversation

@bagder

@bagder bagder commented May 12, 2026

Copy link
Copy Markdown
Member

Previously, the use of checkprefix() alone allowed the code to match not only on "blksize" but also (mistakenly) on "blksizeFOO" etc.

Reported-by: Andrew Nesbit

Previously, the use of checkprefix() alone allowed the code to match not
only on "blksize" but also (mistakenly) on "blksizeFOO" etc.

Reported-by: Andrew Nesbit
@github-actions github-actions Bot added the TFTP label May 12, 2026
@bagder bagder requested a review from Copilot May 12, 2026 07:28
@bagder bagder marked this pull request as ready for review May 12, 2026 07:28

Copilot AI 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.

Pull request overview

Tightens TFTP OACK option parsing in lib/tftp.c so option names must match exactly (not just by prefix), preventing unintended matches like "blksizeFOO" being treated as "blksize".

Changes:

  • Compute the received option name length once per loop iteration.
  • Require an exact-length match in addition to checkprefix() for blksize.
  • Require an exact-length match in addition to checkprefix() for tsize.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@testclutch

Copy link
Copy Markdown

Analysis of PR #21560 at 56aea468:

Test 2410 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 10 different CI jobs (the link just goes to one of them). Note that this CI job has had a number of other flaky tests recently (2, to be exact) so it may be that this failure is rather a systemic issue with this job and not with this specific PR.

Generated by Testclutch

@bagder bagder closed this in 2256162 May 12, 2026
@bagder bagder deleted the bagder/tftp-prefix branch May 12, 2026 08:26
outcast36 pushed a commit to greearb/curl that referenced this pull request Jun 3, 2026
Previously, the use of checkprefix() alone allowed the code to match not
only on "blksize" but also (mistakenly) on "blksizeFOO" etc.

Reported-by: Andrew Nesbit
Closes curl#21560
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants