Annotate unpack* function errors with where the error happened. (#1590)

This commit adds annotations to the error message when an error occurs
during unpacking of a DNS message, the annotation will give detiail on
where in the DNS message the unpacking error occured at. This helps to
improve the debugability of such errors.

Co-authored-by: Eric Skoglund <eric.skoglund@internetstiftelsen.se>
This commit is contained in:
Eric Skoglund 2025-07-09 14:08:50 +02:00 committed by GitHub
parent 4669827139
commit 186ccfbcd9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 236 additions and 223 deletions

25
msg.go
View File

@ -872,7 +872,7 @@ func (dns *Msg) unpack(dh Header, msg []byte, off int) (err error) {
// TODO(miek) make this an error?
// use PackOpt to let people tell how detailed the error reporting should be?
// if off != len(msg) {
// // println("dns: extra bytes in dns packet", off, "<", len(msg))
// // println("dns: extra bytes in dns packet", off, "<", len(msg))
// }
return err
}
@ -1123,23 +1123,28 @@ func unpackQuestion(msg []byte, off int) (Question, int, error) {
)
q.Name, off, err = UnpackDomainName(msg, off)
if err != nil {
return q, off, err
return q, off, fmt.Errorf("question.Name: %w", err)
}
if off == len(msg) {
return q, off, nil
}
q.Qtype, off, err = unpackUint16(msg, off)
if err != nil {
return q, off, err
return q, off, fmt.Errorf("question.Qtype: %w", err)
}
if off == len(msg) {
return q, off, nil
}
q.Qclass, off, err = unpackUint16(msg, off)
if err != nil {
return q, off, fmt.Errorf("question.Qclass: %w", err)
}
if off == len(msg) {
return q, off, nil
}
return q, off, err
return q, off, nil
}
func (dh *Header) pack(msg []byte, off int, compression compressionMap, compress bool) (int, error) {
@ -1177,27 +1182,27 @@ func unpackMsgHdr(msg []byte, off int) (Header, int, error) {
)
dh.Id, off, err = unpackUint16(msg, off)
if err != nil {
return dh, off, err
return dh, off, fmt.Errorf("header.Id: %w", err)
}
dh.Bits, off, err = unpackUint16(msg, off)
if err != nil {
return dh, off, err
return dh, off, fmt.Errorf("header.Bits: %w", err)
}
dh.Qdcount, off, err = unpackUint16(msg, off)
if err != nil {
return dh, off, err
return dh, off, fmt.Errorf("header.Qdcount: %w", err)
}
dh.Ancount, off, err = unpackUint16(msg, off)
if err != nil {
return dh, off, err
return dh, off, fmt.Errorf("header.Ancount: %w", err)
}
dh.Nscount, off, err = unpackUint16(msg, off)
if err != nil {
return dh, off, err
return dh, off, fmt.Errorf("header.Nscount: %w", err)
}
dh.Arcount, off, err = unpackUint16(msg, off)
if err != nil {
return dh, off, err
return dh, off, fmt.Errorf("header.Arcount: %w", err)
}
return dh, off, nil
}

View File

@ -24,6 +24,7 @@ var packageHdr = `
package dns
import "fmt"
`
// getTypeStruct will take a type and the package scope, and return the
@ -208,10 +209,15 @@ _ = rdStart
for i := 1; i < st.NumFields(); i++ {
o := func(s string) {
fmt.Fprintf(b, s, st.Field(i).Name())
fmt.Fprint(b, `if err != nil {
return off, err
err_name := name
if name != st.Field(i).Name() {
err_name += "." + st.Field(i).Name()
}
fmt.Fprintf(b, `if err != nil {
return off, fmt.Errorf("%s: %%w", err)
}
`)
`, err_name)
}
// size-* are special, because they reference a struct member we should use for the length.

422
zmsg.go

File diff suppressed because it is too large Load Diff