net/icmp: Echo.ID and Echo.Seq are defined as int, but Marshal converts them to uint16

[Yuen Sun]

Description

In the Go net/icmp package, the ID and Seq fields of the Echo struct are defined as int types. However, in the Marshal method, these fields are cast to uint16. This can lead to data truncation if the values of ID or Seq exceed the 16-bit range.

// An Echo represents an ICMP echo request or reply message body.
type Echo struct {
ID int // identifier
Seq int // sequence number
Data []byte // data
}

// Len implements the Len method of MessageBody interface.
func (p *Echo) Len(proto int) int {
if p == nil {
return 0
}
return 4 + len(p.Data)
}

// Marshal implements the Marshal method of MessageBody interface.
func (p *Echo) Marshal(proto int) ([]byte, error) {
b := make([]byte, 4+len(p.Data))
binary.BigEndian.PutUint16(b[:2], uint16(p.ID))
binary.BigEndian.PutUint16(b[2:4], uint16(p.Seq))
copy(b[4:], p.Data)
return b, nil
}

Design Intent

I noticed that the ID and Seq fields are defined as int, which is not strictly aligned with the ICMP protocol specification (RFC 792) that requires these fields to be 16-bit. Could you please clarify the original intent behind defining these fields as int instead of uint16? Specifically:

• What was the rationale for using int instead of uint16?
• Was there a specific use case or flexibility in mind that required int?

Summary

I believe changing the types of ID and Seq to uint16 would make the implementation more consistent with the ICMP protocol specification. Understanding the original design intent would also help the community better align with Go's design philosophy.

Thank you for your attention!

[Jason E. Aten]

Hi Yuen Sun,

Your question is a fine one, but better directed to the Go development team

than to general users of Go. If its going to be fixed, it will need issue anyway, so

I suggest simply filing an issue and/or asking on golang-dev.

https://github.com/golang/go/issues

https://groups.google.com/g/golang-dev

Best wishes,

Jason

[Ian Lance Taylor]

On Sun, May 11, 2025 at 8:59 AM Yuen Sun <joon...@gmail.com> wrote:
>
> I believe changing the types of ID and Seq to uint16 would make the implementation more consistent with the ICMP protocol specification. Understanding the original design intent would also help the community better align with Go's design philosophy.

Making such a change could break existing users. Maybe it would have
been a good idea initially, but it's not something we can change now.

Ian