Skip to content

Conversation

thorsager
Copy link
Contributor

Prior to this int64 have been used to store the values, this requires some, perhaps unsafe "truncation" of contentLength when mapping to the internal buffer for storing payload.

I have changed the internal type to better fit the usage of the values, and added guard checks when parsing.

To better test for parsing-errors I have refactored testing.

Prior to this int64 have been used to store the values, this requires
some, perhaps unsafe "truncation" of contentLength when mapping to the internal buffer for storing payload.

I have changed the internal type to better fit the usage of the values, and added guard checks when parsing.

To better test for parsing-errors I have refactored testing.
@thorsager thorsager marked this pull request as ready for review May 31, 2024 07:09
@mosajjal
Copy link
Contributor

mosajjal commented Jun 2, 2024

@thorsager thanks for this. since we don't use testify in this lib (yet), I commented them out and replaced them with a basic equivalent using stdlib. can you confirm if the tests still work?

@mosajjal mosajjal self-assigned this Jun 2, 2024
@thorsager
Copy link
Contributor Author

@thorsager thanks for this. since we don't use testify in this lib (yet), I commented them out and replaced them with a basic equivalent using stdlib. can you confirm if the tests still work?

It looks as though the Subset tests are not working as expected, I'll have a look and update :) .. Thank you for getting this started.. I hop to get it done later today :)

@thorsager
Copy link
Contributor Author

Sorry for the delay ..

I have done a bit of cleanup, adding some helper functions, to allow for more convenience when writing the actual tests
.. have a look

@thorsager
Copy link
Contributor Author

@mosajjal anything I can do to move this one along?

@mosajjal
Copy link
Contributor

been too busy to take a look at this repo lately. hopefully I'll get some time over the weekend to review the code.

@thorsager
Copy link
Contributor Author

No worries 💪🙂

@mosajjal mosajjal merged commit 9b0292c into gopacket:master Aug 23, 2024
@mosajjal
Copy link
Contributor

good stuff. merged!

@thorsager thorsager deleted the integer-conversion branch August 24, 2024 15:26
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.

2 participants