Skip to content

fix: use comma-ok type assertion and length guard in parseNtpTimestamp#1590

Open
Aprazor wants to merge 2 commits intoprometheus:mainfrom
Aprazor:fix/ntp-timestamp-type-assertion-panic
Open

fix: use comma-ok type assertion and length guard in parseNtpTimestamp#1590
Aprazor wants to merge 2 commits intoprometheus:mainfrom
Aprazor:fix/ntp-timestamp-type-assertion-panic

Conversation

@Aprazor
Copy link
Copy Markdown
Contributor

@Aprazor Aprazor commented Mar 21, 2026

The bare type assertion pdu.Value.([]byte) panics if the PDU value
is not []byte. The subsequent data[:4] and data[4:] slicing panics
if data is shorter than 8 bytes. Add proper type check and length
validation with descriptive error messages.

The bare type assertion pdu.Value.([]byte) panics if the PDU value
is not []byte. The subsequent data[:4] and data[4:] slicing panics
if data is shorter than 8 bytes. Add proper type check and length
validation with descriptive error messages.

Signed-off-by: Aprazors <Aprazors@gmail.com>
@Aprazor Aprazor force-pushed the fix/ntp-timestamp-type-assertion-panic branch from 69c942c to 7465db7 Compare March 21, 2026 17:59
Copy link
Copy Markdown
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a test case.

Per reviewer feedback, add test cases for:
- invalid type (integer instead of []byte)
- too-short data (less than 8 bytes)

Both cases verify the error is returned instead of panicking.

Signed-off-by: Aprazors <Aprazors@gmail.com>
@Aprazor
Copy link
Copy Markdown
Contributor Author

Aprazor commented Mar 24, 2026

added test cases for the invalid type and too-short data paths. should be good now.

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