Skip to content

Exception when trying to parse an IPv6 address #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Jerome-PS wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Exception when trying to parse an IPv6 address #53

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
972f0f9
Exception when trying to parse an IPv6 address
Jerome-PS Apr 13, 2023
a67ec6d
Return int directly
Jerome-PS Apr 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pcapng/structs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,7 @@ def _decode_value(self, value, ftype):
if ftype == TYPE_IPV6_PREFIX:
return (
unpack_ipv6(value[:16]),
struct.unpack(self.endianness + "B", value[16]),
struct.unpack(self.endianness + "B", value[16:17]),

@Flameeyes Flameeyes Apr 16, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't sound right, among other things because "B" cannot have endianness (it's a single byte) and it shouldn't unpack two bytes, but just one.

I would suggest changing this to just be value[16] without the unpack then.

This should be options valid for the interface description block: https://ietf-opsawg-wg.github.io/draft-ietf-opsawg-pcap/draft-ietf-opsawg-pcapng.html#name-interface-description-block (because it matches

python-pcapng/pcapng/blocks.py

Line 277 in 3e13e64

Option(5, "if_IPv6addr", "ipv6+prefix", multiple=True),
)

And that option is documented as:

The if_IPv6addr option is an IPv6 network address and corresponding prefix length for the interface. The first 16 octets are the IP address and the next octet is the prefix length. This option can be repeated multiple times within the same Interface Description Block when multiple IPv6 addresses are assigned to the interface.

since there's a single octect there's nothing to unpack at all there.

@Jerome-PS Jerome-PS Apr 16, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

Yes, sure, I am not the one who wrote the unpack. I just thought I'd change as little as possible, you should be able to return value[16]. What was faulty was to call unpack with an integer and not a byte array and that caused an exception. As long as there is not exception any more and the integer value is return properly, it's OK.

Oh by the way, you are wrong if you think value[16:17] creates a 2-byte array, it only creates a 1-byte array. value[16:16] creates a 0-byte array. (I know I myself also sometimes get confused about basic python although I use it quite a lot). See: https://docs.python.org/3/library/stdtypes.html#typesseq

4. The slice of s from i to j is defined as the sequence of items with index k such that i <= k < j.

Cheers.

@Flameeyes Flameeyes Apr 16, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gha I clearly need to have more coffee before reviewing code, sorry yes it's obvious now that I re-read them.

But yeah please change this to just return the octet, using the unpack on it will just throw the next person reading the code off.

@Jerome-PS Jerome-PS Apr 17, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you see my new commit?

)

if ftype == TYPE_MACADDR:
Expand Down