Skip to content

Commit 5322218

Browse files
committed
SA1026, SA5008: don't loop forever when marshaling cyclic pointer type
Closes gh-1202 (cherry picked from commit 4a64968)
1 parent d0c6d29 commit 5322218

File tree

4 files changed

+40
-0
lines changed

4 files changed

+40
-0
lines changed

staticcheck/fakexml/marshal.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,15 @@ var textMarshalerType = types.NewInterfaceType([]*types.Func{
113113

114114
var N = 0
115115

116+
type CyclicTypeError struct {
117+
Type types.Type
118+
Path string
119+
}
120+
121+
func (err *CyclicTypeError) Error() string {
122+
return "cyclic type"
123+
}
124+
116125
// marshalValue writes one or more XML elements representing val.
117126
// If val was obtained from a struct field, finfo must have its details.
118127
func (e *Encoder) marshalValue(val fakereflect.TypeAndCanAddr, finfo *fieldInfo, startTemplate *StartElement, stack string) error {
@@ -122,11 +131,17 @@ func (e *Encoder) marshalValue(val fakereflect.TypeAndCanAddr, finfo *fieldInfo,
122131
e.seen[val] = struct{}{}
123132

124133
// Drill into interfaces and pointers.
134+
seen := map[fakereflect.TypeAndCanAddr]struct{}{}
125135
for val.IsInterface() || val.IsPtr() {
126136
if val.IsInterface() {
127137
return nil
128138
}
129139
val = val.Elem()
140+
if _, ok := seen[val]; ok {
141+
// Loop in type graph, e.g. 'type P *P'
142+
return &CyclicTypeError{val.Type, stack}
143+
}
144+
seen[val] = struct{}{}
130145
}
131146

132147
// Check for marshaler.

staticcheck/fakexml/typeinfo.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,14 @@ func StructFieldInfo(f fakereflect.StructField) (*fieldInfo, error) {
249249
// in case it exists and has a valid xml field tag, otherwise
250250
// it returns nil.
251251
func lookupXMLName(typ fakereflect.TypeAndCanAddr) (xmlname *fieldInfo) {
252+
seen := map[fakereflect.TypeAndCanAddr]struct{}{}
252253
for typ.IsPtr() {
253254
typ = typ.Elem()
255+
if _, ok := seen[typ]; ok {
256+
// Loop in type graph, e.g. 'type P *P'
257+
return nil
258+
}
259+
seen[typ] = struct{}{}
254260
}
255261
if !typ.IsStruct() {
256262
return nil

staticcheck/lint.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -897,6 +897,13 @@ func checkUnsupportedMarshalXML(call *Call) {
897897
} else {
898898
arg.Invalid(fmt.Sprintf("trying to marshal unsupported type %s, via %s", typ, err.Path))
899899
}
900+
case *fakexml.CyclicTypeError:
901+
typ := types.TypeString(err.Type, types.RelativeTo(arg.Value.Value.Parent().Pkg.Pkg))
902+
if err.Path == "x" {
903+
arg.Invalid(fmt.Sprintf("trying to marshal cyclic type %s", typ))
904+
} else {
905+
arg.Invalid(fmt.Sprintf("trying to marshal cyclic type %s, via %s", typ, err.Path))
906+
}
900907
case *fakexml.TagPathError:
901908
// Vet does a better job at reporting this error, because it can flag the actual struct tags, not just the call to Marshal
902909
default:

staticcheck/testdata/src/CheckUnsupportedMarshal/CheckUnsupportedMarshal.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,3 +303,15 @@ func toplevelPointer() {
303303
xml.Marshal(ToplevelPointerMarshalerXML{}) // want `unsupported type`
304304
xml.Marshal(ToplevelPointerMarshalerText{}) // want `unsupported type`
305305
}
306+
307+
func cyclicPointer() {
308+
type P *P
309+
type S2 struct {
310+
Bar P
311+
}
312+
type S1 struct {
313+
Foo S2
314+
}
315+
var s S1
316+
xml.Marshal(s) // want `cyclic type P, via x\.Foo\.Bar`
317+
}

0 commit comments

Comments
 (0)