.net core binary serialization fails about packetnet HOT 8 CLOSED

from packetnet.

It seems Lazy isn't serializable anymore in .NET Core. I'm not sure how useful [Serializable] is in the library itself though (which should then be removed from everything), as I think you only want to save the underlying byte array (which can be used to reconstruct the packets again anyway).

from packetnet.

from packetnet.

I was thinking about removing serialization tests and also the Serializable attribute from all classes in main project. I don't really see a good use case for that as the properties are based on byte array anyway and all you really need is to serialize is that. The only exception to the removal of all Serializable attributes is perhaps the ByteArraySegment as I can see the benefit of saving Length/Offset.

from packetnet.

from packetnet.

The tests are OK. Lazy just isn't serializable in .NET Core, which means that the packet serialization isn't going to work either (for the same reason). I think removing the Serializable attribute would be OK, I don't think many will use it as you're much better of serializing the ByteArraySegment or the underlying array.

The downside of simply removing all binary serialization tests is that most of these test methods are the only thing kind of validating the pcap files and their contained packet. So perhaps these should be changed to assert the expected value rather than comparing serialized vs deserialized. For example, this https://github.com/chmorgan/packetnet/blob/2f2f4208f2ee5efd707360bae969f64e7f198116/Test/PacketType/IPPacketTest.cs#L74 could check if ip.Protocol is ProtocolType.IPv4 (didn't check what it actually is). This is of course more work than removing these tests completely, but then there's at least some tests for packet correctness (expected values can be found in Wireshark). I don't have a lot of free time at the moment, so I hope this makes sense 👍

from packetnet.

@Phyxion I took a look and didn't see anything that made sense to preserve. I did have one question. You flagged Echo as obsolete. Should we drop support for that option from TcpPacket.cs? Otherwise there are warnings printed during the compile. Thoughts?

from packetnet.

The Echo was already flagged as Obsolete. I've not removed it yet because while RFC states it's obsolete, I'm not sure if it needs to be removed from the library. Perhaps it would be better to drop the Obsolete attribute instead. Also created PR #96 for these changes so it's easier to keep track on and allow AppVeyor to show results :)

from packetnet.