Skip to content

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

@tklauser

Description

@tklauser

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

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 04f6565 (PR #660), namely the following change:

@@ -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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions