github-vet / github-vet/rangeloop-pointer-findings

bosun-monitor/bosun: cmd/bosun/expr/azureai.go; 88 LoC

Open
#15,107 0 comments 0 reactions 0 assignees View on GitHub
fresh medium
Dominant language
No language data
Stars
0
Forks
0
PR merge metrics
PR metrics pending

Description

Found a possible issue in [bosun-monitor/bosun](https://www.github.com/bosun-monitor/bosun) at [cmd/bosun/expr/azureai.go](https://github.com/bosun-monitor/bosun/blob/4ea808ab2ddfcd7dcc135372ea217aa17ef764cc/cmd/bosun/expr/azureai.go#L92-L179)

Below is the message reported by the analyzer for this snippet of code. Beware that the analyzer only reports the first issue it finds, so please do not limit your consideration to the contents of the below message.

> reference to seg is reassigned at line 143

[Click here to see the code in its original context.](https://github.com/bosun-monitor/bosun/blob/4ea808ab2ddfcd7dcc135372ea217aa17ef764cc/cmd/bosun/expr/azureai.go#L92-L179)

Click here to show the 88 line(s) of Go which triggered the analyzer.

```go
for _, seg := range *res.Value.Segments {
handleInnerSegment := func(s ainsights.MetricsSegmentInfo) error {
met, ok := s.AdditionalProperties[metric]
if !ok {
return fmt.Errorf("expected additional properties not found on inner segment while handling azure query")
}
metMap, ok := met.(map[string]interface{})
if !ok {
return fmt.Errorf("unexpected type for additional properties not found on inner segment while handling azure query")
}
metVal, ok := metMap[agtype]
if !ok {
return fmt.Errorf("expected aggregation value for aggregation %v not found on inner segment while handling azure query", agtype)
}
tags := opentsdb.TagSet{}
if hasSegments {
key := string(segments[segLen-1])
val, ok := s.AdditionalProperties[key]
if !ok {
return fmt.Errorf("unexpected dimension/segment key %v not found in response", key)
}
sVal, ok := val.(string)
if !ok {
return fmt.Errorf("unexpected dimension/segment value for key %v in response", key)
}
tags[key] = sVal
}
tags = tags.Merge(basetags)
err := tags.Clean()
if err != nil {
return err
}
if _, ok := seriesMap[tags.Tags()]; !ok {
seriesMap[tags.Tags()] = make(Series)
}
if v, ok := metVal.(float64); ok && seg.Start != nil {
seriesMap[tags.Tags()][seg.Start.Time] = v
}
return nil
}

// Simple case with no Segments/Dimensions
if !hasSegments {
err := handleInnerSegment(seg)
if err != nil {
return r, err
}
continue
}

// Case with Segments/Dimensions
next := &seg
// decend (fast forward) to the next nested MetricsSegmentInfo by moving the 'next' pointer
decend := func(dim string) error {
if next == nil || next.Segments == nil || len(*next.Segments) == 0 {
return fmt.Errorf("unexpected insights response while handling dimension %s", dim)
}
next = &(*next.Segments)[0]
return nil
}
if segLen > 1 {
if err := decend("root-level"); err != nil {
return r, err
}
}
// When multiple dimensions are requests, there are nested MetricsSegmentInfo objects
// The higher levels just contain all the dimension key-value pairs except the last.
// So we fast forward to the depth that has the last tag pair and the metric values
// collect tags along the way
for i := 0; i < segLen-1; i++ {
segStr := string(segments[i])
basetags[segStr] = next.AdditionalProperties[segStr].(string)
if i != segLen-2 { // the last dimension/segment will be in same []MetricsSegmentInfo slice as the metric value
if err := decend(string(segments[i])); err != nil {
return r, err
}
}
}
if next == nil {
return r, fmt.Errorf("unexpected segement/dimension in insights response")
}
for _, innerSeg := range *next.Segments {
err := handleInnerSegment(innerSeg)
if err != nil {
return r, err
}
}
}

```

Leave a reaction on this issue to contribute to the project by classifying this instance as a **Bug** :-1:, **Mitigated** :+1:, or **Desirable Behavior** :rocket:
See the descriptions of the classifications [here](https://github.com/github-vet/rangeclosure-findings#how-can-i-help) for more information.

commit ID: 4ea808ab2ddfcd7dcc135372ea217aa17ef764cc

Contributor guide

No contributing guide indexed for this repository

Research direction

Read cmd/bosun/expr/azureai.go around lines 92-179, starting with the range loop and the pointer used for nested segments. Trace how Azure response segments are traversed and whether reusing the range variable can alter the referenced data. Done means determining whether the analyzer finding causes incorrect behavior and classifying it as Bug, Mitigated, or Desirable Behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.