Repository: scionproto/scion
Pull request #3534 (2019-12-17 08:00:36)

 Consistently handle nil in Copy implementations 
Check for `nil`-receiver in `Copy`.

The trigger for this change is a recently introduced small bug in `snet.Dial` where copying a `nil`-Path causes a panic.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/scionproto/scion/3534)
<!-- Reviewable:end -->



Patch:
diff --git a/go/lib/addr/appaddr.go b/go/lib/addr/appaddr.go
index 3111f1b8c..8333695c2 100644
--- a/go/lib/addr/appaddr.go
+++ b/go/lib/addr/appaddr.go
@@ -36,6 +36,9 @@ func AppAddrFromUDP(a *net.UDPAddr) *AppAddr {
 }
 
 func (a *AppAddr) Copy() *AppAddr {
+	if a == nil {
+		return nil
+	}
 	var l3 HostAddr
 	if a.L3 != nil {
 		l3 = a.L3.Copy()
diff --git a/go/lib/ctrl/ctrl.go b/go/lib/ctrl/ctrl.go
index 1439750b1..4ead175e5 100644
--- a/go/lib/ctrl/ctrl.go
+++ b/go/lib/ctrl/ctrl.go
@@ -103,6 +103,9 @@ func (p *Pld) Len() int {
 }
 
 func (p *Pld) Copy() (*Pld, error) {
+	if p == nil {
+		return nil, nil
+	}
 	raw, err := proto.PackRoot(p)
 	if err != nil {
 		return nil, err
diff --git a/go/lib/ctrl/seg/seg.go b/go/lib/ctrl/seg/seg.go
index e6631e96f..089de972e 100644
--- a/go/lib/ctrl/seg/seg.go
+++ b/go/lib/ctrl/seg/seg.go
@@ -382,6 +382,9 @@ func (ps *PathSegment) validateIdx(idx int) error {
 
 // ShallowCopy creates a shallow copy of the path segment.
 func (ps *PathSegment) ShallowCopy() *PathSegment {
+	if ps == nil {
+		return nil
+	}
 	rawEntries := make([]*proto.SignedBlobS, len(ps.RawASEntries))
 	copy(rawEntries, ps.RawASEntries)
 	entries := make([]*ASEntry, len(ps.ASEntries))
diff --git a/go/lib/l4/udp.go b/go/lib/l4/udp.go
index 0ec436c5e..e28d1bc93 100644
--- a/go/lib/l4/udp.go
+++ b/go/lib/l4/udp.go
@@ -103,6 +103,9 @@ func (u *UDP) SetPldLen(pldLen int) {
 }
 
 func (u *UDP) Copy() L4Header {
+	if u == nil {
+		return nil
+	}
 	return &UDP{u.SrcPort, u.DstPort, u.TotalLen, append(common.RawBytes(nil), u.Checksum...)}
 }
 
diff --git a/go/lib/layers/extensions.go b/go/lib/layers/extensions.go
index 774244be5..bf3313e0a 100644
--- a/go/lib/layers/extensions.go
+++ b/go/lib/layers/extensions.go
@@ -149,6 +149,9 @@ func ExtnSCMPFromRaw(b common.RawBytes) (*ExtnSCMP, error) {
 }
 
 func (e *ExtnSCMP) Copy() common.Extension {
+	if e == nil {
+		return nil
+	}
 	return &ExtnSCMP{Error: e.Error, HopByHop: e.HopByHop}
 }
 
diff --git a/go/lib/sciond/types.go b/go/lib/sciond/types.go
index 81c52f23b..cbb198b85 100644
--- a/go/lib/sciond/types.go
+++ b/go/lib/sciond/types.go
@@ -130,6 +130,9 @@ type PathReq struct {
 }
 
 func (pathReq *PathReq) Copy() *PathReq {
+	if pathReq == nil {
+		return nil
+	}
 	return &PathReq{
 		Dst:      pathReq.Dst,
 		Src:      pathReq.Src,
diff --git a/go/lib/scmp/hdr.go b/go/lib/scmp/hdr.go
index 6a75d6c1f..6584b0a79 100644
--- a/go/lib/scmp/hdr.go
+++ b/go/lib/scmp/hdr.go
@@ -121,6 +121,9 @@ func (h *Hdr) L4Len() int {
 func (h *Hdr) Reverse() {}
 
 func (h *Hdr) Copy() l4.L4Header {
+	if h == nil {
+		return nil
+	}
 	return &Hdr{
 		h.Class, h.Type, h.TotalLen, append(common.RawBytes(nil), h.Checksum...), h.Timestamp,
 	}
diff --git a/go/lib/scmp/info.go b/go/lib/scmp/info.go
index ae9632a99..7dec22560 100644
--- a/go/lib/scmp/info.go
+++ b/go/lib/scmp/info.go
@@ -76,6 +76,9 @@ func InfoEchoFromRaw(b common.RawBytes) (*InfoEcho, error) {
 }
 
 func (e *InfoEcho) Copy() Info {
+	if e == nil {
+		return nil
+	}
 	return &InfoEcho{Id: e.Id, Seq: e.Seq}
 }
 
@@ -110,6 +113,9 @@ func InfoPktSizeFromRaw(b common.RawBytes) (*InfoPktSize, error) {
 }
 
 func (p *InfoPktSize) Copy() Info {
+	if p == nil {
+		return nil
+	}
 	return &InfoPktSize{Size: p.Size, MTU: p.MTU}
 }
 
@@ -160,6 +166,9 @@ func InfoPathOffsetsFromRaw(b common.RawBytes) (*InfoPathOffsets, error) {
 }
 
 func (p *InfoPathOffsets) Copy() Info {
+	if p == nil {
+		return nil
+	}
 	return &InfoPathOffsets{InfoF: p.InfoF, HopF: p.HopF, IfID: p.IfID, Ingress: p.Ingress}
 }
 
@@ -213,6 +222,9 @@ func InfoRevocationFromRaw(b common.RawBytes) (*InfoRevocation, error) {
 	return p, nil
 }
 func (r *InfoRevocation) Copy() Info {
+	if r == nil {
+		return nil
+	}
 	return &InfoRevocation{
 		r.InfoPathOffsets.Copy().(*InfoPathOffsets),
 		append(common.RawBytes(nil), r.RawSRev...),
@@ -257,6 +269,9 @@ func InfoExtIdxFromRaw(b common.RawBytes) (*InfoExtIdx, error) {
 }
 
 func (e *InfoExtIdx) Copy() Info {
+	if e == nil {
+		return nil
+	}
 	return &InfoExtIdx{Idx: e.Idx}
 }
 
diff --git a/go/lib/scmp/info_recordpath.go b/go/lib/scmp/info_recordpath.go
index 016257d7d..6a9a4fc91 100644
--- a/go/lib/scmp/info_recordpath.go
+++ b/go/lib/scmp/info_recordpath.go
@@ -125,6 +125,9 @@ func InfoRecordPathFromRaw(b common.RawBytes) (*InfoRecordPath, error) {
 }
 
 func (rec *InfoRecordPath) Copy() Info {
+	if rec == nil {
+		return nil
+	}
 	r := &InfoRecordPath{Id: rec.Id}
 	r.Entries = make([]*RecordPathEntry, rec.NumHops(), rec.TotalHops())
 	for i, e := range rec.Entries {
diff --git a/go/lib/scmp/info_traceroute.go b/go/lib/scmp/info_traceroute.go
index 3933a91c3..dc634b749 100644
--- a/go/lib/scmp/info_traceroute.go
+++ b/go/lib/scmp/info_traceroute.go
@@ -70,6 +70,9 @@ func InfoTraceRouteFromRaw(b common.RawBytes) (*InfoTraceRoute, error) {
 }
 
 func (e *InfoTraceRoute) Copy() Info {
+	if e == nil {
+		return nil
+	}
 	return &InfoTraceRoute{Id: e.Id, IfID: e.IfID, IA: e.IA, HopOff: e.HopOff, In: e.In}
 }
 
diff --git a/go/lib/scmp/meta.go b/go/lib/scmp/meta.go
index 5fe9447f4..10153a204 100644
--- a/go/lib/scmp/meta.go
+++ b/go/lib/scmp/meta.go
@@ -50,6 +50,9 @@ func MetaFromRaw(b []byte) (*Meta, error) {
 }
 
 func (m *Meta) Copy() *Meta {
+	if m == nil {
+		return nil
+	}
 	return &Meta{
 		InfoLen: m.InfoLen, CmnHdrLen: m.CmnHdrLen, AddrHdrLen: m.AddrHdrLen,
 		PathHdrLen: m.PathHdrLen, ExtHdrsLen: m.ExtHdrsLen,
diff --git a/go/lib/scmp/pld.go b/go/lib/scmp/pld.go
index 9dd9922d1..00a539ffd 100644
--- a/go/lib/scmp/pld.go
+++ b/go/lib/scmp/pld.go
@@ -85,6 +85,9 @@ func PldFromQuotes(ct ClassType, info Info, l4 common.L4ProtocolType, f QuoteFun
 }
 
 func (p *Payload) Copy() (common.Payload, error) {
+	if p == nil {
+		return nil, nil
+	}
 	c := &Payload{}
 	c.Meta = p.Meta.Copy()
 	c.Info = p.Info
diff --git a/go/lib/scrypto/cert/cert.go b/go/lib/scrypto/cert/cert.go
index 9f6603f2c..52a4712f8 100644
--- a/go/lib/scrypto/cert/cert.go
+++ b/go/lib/scrypto/cert/cert.go
@@ -177,6 +177,9 @@ func (c *Certificate) sigPack() (common.RawBytes, error) {
 }
 
 func (c *Certificate) Copy() *Certificate {
+	if c == nil {
+		return nil
+	}
 	n := &Certificate{
 		CanIssue:       c.CanIssue,
 		Comment:        c.Comment,
diff --git a/go/lib/scrypto/cert/chain.go b/go/lib/scrypto/cert/chain.go
index 0b593fa63..70654273c 100644
--- a/go/lib/scrypto/cert/chain.go
+++ b/go/lib/scrypto/cert/chain.go
@@ -214,6 +214,9 @@ func (c *Chain) Compress() (common.RawBytes, error) {
 }
 
 func (c *Chain) Copy() *Chain {
+	if c == nil {
+		return nil
+	}
 	return &Chain{Issuer: c.Issuer.Copy(), Leaf: c.Leaf.Copy()}
 }
 
diff --git a/go/lib/snet/path.go b/go/lib/snet/path.go
index 216fd3794..9d54b1de7 100644
--- a/go/lib/snet/path.go
+++ b/go/lib/snet/path.go
@@ -113,6 +113,9 @@ func (p *partialPath) Expiry() time.Time {
 }
 
 func (p *partialPath) Copy() Path {
+	if p == nil {
+		return nil
+	}
 	return &partialPath{
 		spath:       p.spath.Copy(),
 		overlay:     copyUDP(p.overlay),
diff --git a/go/lib/spath/path.go b/go/lib/spath/path.go
index 7e40d0d67..3d909b3c9 100644
--- a/go/lib/spath/path.go
+++ b/go/lib/spath/path.go
@@ -69,6 +69,9 @@ func NewOneHop(isd addr.ISD, ifid common.IFIDType, ts time.Time, exp ExpTimeType
 }
 
 func (p *Path) Copy() *Path {
+	if p == nil {
+		return nil
+	}
 	return &Path{append(common.RawBytes(nil), p.Raw...), p.InfOff, p.HopOff}
 }
 
diff --git a/go/lib/spkt/spkt.go b/go/lib/spkt/spkt.go
index 3ce9eaeba..647ffe893 100644
--- a/go/lib/spkt/spkt.go
+++ b/go/lib/spkt/spkt.go
@@ -36,31 +36,24 @@ type ScnPkt struct {
 }
 
 func (s *ScnPkt) Copy() (*ScnPkt, error) {
+	if s == nil {
+		return nil, nil
+	}
 	var err error
 	c := &ScnPkt{}
 	c.DstIA = s.DstIA
 	c.SrcIA = s.SrcIA
-	if s.DstHost != nil {
-		c.DstHost = s.DstHost.Copy()
-	}
-	if s.SrcHost != nil {
-		c.SrcHost = s.SrcHost.Copy()
-	}
-	if s.Path != nil {
-		c.Path = s.Path.Copy()
-	}
+	c.DstHost = s.DstHost.Copy()
+	c.SrcHost = s.SrcHost.Copy()
+	c.Path = s.Path.Copy()
 	for _, e := range s.HBHExt {
 		c.HBHExt = append(c.HBHExt, e.Copy())
 	}
 	for _, e := range s.E2EExt {
 		c.E2EExt = append(c.E2EExt, e.Copy())
 	}
-	if s.L4 != nil {
-		c.L4 = s.L4.Copy()
-	}
-	if s.Pld != nil {
-		c.Pld, err = s.Pld.Copy()
-	}
+	c.L4 = s.L4.Copy()
+	c.Pld, err = s.Pld.Copy()
 	return c, err
 }
 
diff --git a/go/lib/spse/scmp_auth/drkey.go b/go/lib/spse/scmp_auth/drkey.go
index 57edd4dcf..b50d60ed1 100644
--- a/go/lib/spse/scmp_auth/drkey.go
+++ b/go/lib/spse/scmp_auth/drkey.go
@@ -132,6 +132,9 @@ func (s *DRKeyExtn) Pack() (common.RawBytes, error) {
 }
 
 func (s *DRKeyExtn) Copy() common.Extension {
+	if s == nil {
+		return nil
+	}
 	c := NewDRKeyExtn()
 	c.Direction = s.Direction
 	copy(c.MAC, s.MAC)
diff --git a/go/lib/spse/scmp_auth/hashtree.go b/go/lib/spse/scmp_auth/hashtree.go
index 5556af0e4..ca721203b 100644
--- a/go/lib/spse/scmp_auth/hashtree.go
+++ b/go/lib/spse/scmp_auth/hashtree.go
@@ -134,6 +134,9 @@ func (s *HashTreeExtn) Pack() (common.RawBytes, error) {
 }
 
 func (s *HashTreeExtn) Copy() common.Extension {
+	if s == nil {
+		return nil
+	}
 	c, _ := NewHashTreeExtn(s.Height)
 	copy(c.Order, s.Order)
 	copy(c.Signature, s.Signature)
diff --git a/go/lib/spse/spse.go b/go/lib/spse/spse.go
index 3cba4bdf4..bca731567 100644
--- a/go/lib/spse/spse.go
+++ b/go/lib/spse/spse.go
@@ -188,6 +188,9 @@ func (s *Extn) Pack() (common.RawBytes, error) {
 }
 
 func (s *Extn) Copy() common.Extension {
+	if s == nil {
+		return nil
+	}
 	c, _ := NewExtn(s.SecMode)
 	copy(c.Metadata, s.Metadata)
 	copy(c.Authenticator, s.Authenticator)
diff --git a/go/lib/svc/resolver.go b/go/lib/svc/resolver.go
index 6da720f6a..90889b77e 100644
--- a/go/lib/svc/resolver.go
+++ b/go/lib/svc/resolver.go
@@ -202,6 +202,9 @@ func (p *path) Expiry() time.Time {
 }
 
 func (p *path) Copy() snet.Path {
+	if p == nil {
+		return nil
+	}
 	return &path{
 		spath:       p.spath.Copy(),
 		overlay:     snet.CopyUDPAddr(p.overlay),
diff --git a/go/sig/internal/snetmigrate/path.go b/go/sig/internal/snetmigrate/path.go
index 7a845351a..67a677c43 100644
--- a/go/sig/internal/snetmigrate/path.go
+++ b/go/sig/internal/snetmigrate/path.go
@@ -57,6 +57,9 @@ func (p *emptyPath) Expiry() time.Time {
 }
 
 func (p *emptyPath) Copy() snet.Path {
+	if p == nil {
+		return nil
+	}
 	return &emptyPath{
 		source: p.source,
 	}


Test patch:
diff --git a/go/lib/xtest/graph/graph.go b/go/lib/xtest/graph/graph.go
index 2688aef24..b4bce0937 100644
--- a/go/lib/xtest/graph/graph.go
+++ b/go/lib/xtest/graph/graph.go
@@ -349,6 +349,9 @@ func newSolution(start addr.IA) *solution {
 }
 
 func (s *solution) Copy() *solution {
+	if s == nil {
+		return nil
+	}
 	newS := &solution{}
 	newS.CurrentIA = s.CurrentIA
 	newS.visited = make(map[addr.IA]struct{})