Skip to content

Return nil error on empty slice in (*DecodingLayerParser).DecodeLayers#847

Open
tklauser wants to merge 1 commit intogoogle:masterfrom
tklauser:decodelayers-empty
Open

Return nil error on empty slice in (*DecodingLayerParser).DecodeLayers#847
tklauser wants to merge 1 commit intogoogle:masterfrom
tklauser:decodelayers-empty

Conversation

@tklauser
Copy link
Contributor

Before v1.1.18 passing an empty data slice to
(*DecodingLayerParser).DecodeLayers would return a nil error. Commit
04f6565 (PR #660) changed this behavior, passing the empty data to the
to return an error from the first layer that considers an empty slice to
be invalid.

Strictly speaking this is an API breakage, so restore the previous
behavior by returning nil straight away on empty data.

Fixes #846

@yerden
Copy link
Contributor

yerden commented Dec 16, 2020

If DecodeLayers returning nil on an empty packet is ok then I think it is more consistent to also update decoded slice to contain no decoded LayerTypes as well.

Regarding whether DecodeLayers should return nil in this case, I don't have a strong opinion. You'd have to check afterwards if len(decoded) != 0 anyway, right?

@tklauser
Copy link
Contributor Author

tklauser commented Dec 16, 2020

If DecodeLayers returning nil on an empty packet is ok then I think it is more consistent to also update decoded slice to contain no decoded LayerTypes as well.

Agree, truncating decoded on an empty packet also matches v1.1.17 behavior. Updated as suggested. I've also updated the test introduced by this PR to check for correct truncation.

Regarding whether DecodeLayers should return nil in this case, I don't have a strong opinion. You'd have to check afterwards if len(decoded) != 0 anyway, right?

Yes, in our code we're already checking that.

Before v1.1.18 passing an empty data slice to
(*DecodingLayerParser).DecodeLayers would return a nil error. Commit
04f6565 (PR google#660) changed this behavior, passing the empty data to the
to return an error from the first layer that considers an empty slice to
be invalid.

Strictly speaking this is an API breakage, so restore the previous
behavior by returning nil straight away on empty data.

Fixes google#846

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants