IP options and TCP

[cristiklein]

Hi,

I added some infrastructure for parsing IP options, with an example of an IP option that I was interested in. I also added support for decoding TCP/IPv4 packets.

Cheers, Cristian

[codecov-io]

Current coverage is 82.10% (diff: 84.46%)

Merging #21 into master will increase coverage by 2.79%

@@             master        #21   diff @@
==========================================
  Files            11         13     +2   
  Lines           411        531   +120   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            326        436   +110   
- Misses           85         95    +10   
  Partials          0          0          

Powered by Codecov. Last update 552472d...9c34cd1

[kisom]

Hello @cristiklein, thanks for the PR. Can you add tests and add yourself to the AUTHORS file?

[cristiklein]

Hello @kisom . Good point. It even helped me find a few bugs in my original contribution. :smile:

[cristiklein]

@kisom Nice catch!

BTW, I hope I'm not opening a can of worms, but I tend to dislike the fact that the payload is stored hexlified, which means that it needs to be divided by two for some length computations. I would rather store everything as original bytes and let the application present the data as it sees fit.

[kisom]

@cristiklein I don't have a strong preference either way; when I first wrote this, the hexlified payload was better for what I was doing (and for debugging). I guess the biggest question now would be whether anyone is relying on the payload being hexlified now.

[cristiklein]

I am leaning towards de-hexlification, due to the fact that upper decoding layers need to call unhexlify, which adds overhead. Then again, I don't currently have a particular need for performance and the library works very well for me as it is now.

There is also the option of a "soft transition". Having a new field, payload_bin that stores bytes and changing payload to a property that hexlifies the bytes on-the-fly. I can prepare a patch, if this solution sounds acceptable.

[kisom]

That sounds like a good way forward. It should probably go in a separate PR, though. Does that work for you?

[cristiklein]

Of course, separate PR. Will open one, once I find some time.

2016-09-08 15:24 GMT+02:00 Kyle Isom notifications@github.com:

That sounds like a good way forward. It should probably go in a separate PR, though. Does that work for you?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kisom/pypcapfile/pull/21#issuecomment-245595621, or mute the thread https://github.com/notifications/unsubscribe-auth/ABlXoSyl6ekmQjg4heJdjdsJsfgQjctmks5qoAxzgaJpZM4Jzevf .

[kisom]

Okay, thanks. I'll merge this now, then.

[kisom]

Follow up: this was released as v0.12.0 and is now on PyPI.