Changed behavior of (*DecodingLayerParser).DecodeLayers on empty data in v1.1.18
- 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
Assessment
This issue has not been assessed yet.