google / google/gopacket

Changed behavior of (*DecodingLayerParser).DecodeLayers on empty data in v1.1.18

Open
#846 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
6.8k
Forks
1.2k
PR merge metrics
No merged PRs in 30d

Description

Before v1.1.18, the following code would not print any message, i.e. `dlp.DecodeLayers` returns a `nil` error:

```go
import (
"fmt"

"github.com/google/gopacket"
"github.com/google/gopacket/layers"
)

func main() {
dlp := gopacket.NewDecodingLayerParser(layer.LayerTypeEthernet, &layers.Ethernet{}, &layers.IPv4{}, &layers.TCP{}, &layersUDP{})
decoded := []gopacket.LayerType{}
empty := []byte{}
err := dlp.DecodeLayers(empty, &decoded)
if err != nil {
fmt.Printf("Error from dlp parser: %v\n", err)
}
}
```

In v1.1.18 this behavior changed and an error is now returned from the first layer, i.e. `layers.Ethernet` in this case:

```
Error from dlp parser: Ethernet packet too small
```

This seems to be due to 04f6565bda0a7c4e84b3b3d6024cab497663674f (PR #660), namely the following change:

```diff
@@ -153,23 +304,15 @@ func (l *DecodingLayerParser) DecodeLayers(data []byte, decoded *[]LayerType) (e
if !l.IgnorePanic {
defer panicToError(&err)
}
- typ := l.first
- *decoded = (*decoded)[:0] // Truncated decoded layers.
- for len(data) > 0 {
- decoder, ok := l.decoders[typ]
- if !ok {
- if l.IgnoreUnsupported {
- return nil
- }
- return UnsupportedLayerType(typ)
- } else if err = decoder.DecodeFromBytes(data, l.df); err != nil {
- return err
+ typ, err := l.decodeFunc(data, decoded)
+ if typ != LayerTypeZero {
+ // no decoder
+ if l.IgnoreUnsupported {
+ return nil
}
- *decoded = append(*decoded, typ)
- typ = decoder.NextLayerType()
- data = decoder.LayerPayload()
+ return UnsupportedLayerType(typ)
}
- return nil
+ return err
}
```

We have code that depends on this behavior and strictly speaking this is an API breakage, so my suggestion would be to keep returning `nil` for empty `data`. The other option of course is to check `len(data) != 0` before calling `(*DecodingLayerParser).DecodeLayers` in our but this would be rather cumbersome.

/cc @notti @yerden

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.