Skip to content

Conversation

@AKOU0
Copy link

@AKOU0 AKOU0 commented Jul 13, 2017

Copies dns.py from vs1, and makes necessary changes to run under vs2.

@AKOU0
Copy link
Author

AKOU0 commented Jul 14, 2017

This PR depends on the following PR to vstruct2: vstruct2 PR 7

@invisig0th
Copy link
Contributor

invisig0th commented Jul 25, 2017

restarting the unit test builds with the newly merged vstruct2 PR... if all goes well, i'll mainline today.

@invisig0th
Copy link
Contributor

Unit tests still failing with merged vstruct2 PR #7.


from __future__ import unicode_literals

try:
Copy link
Contributor

Choose a reason for hiding this comment

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

a construct like this probably belongs in dissect/compat.py ( rather than having a separate issue like this in every protocol module ). Additionally, we should probably have the semantics lean forward toward 3 rather than backward toward 2 ( ie, not use the name xrange ).

Copy link
Author

Choose a reason for hiding this comment

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

moved, and tried to make it more py3-ish.

DNS_FLAG_TRUNC = 1 << 9
DNS_FLAG_RECUR = 1 << 8
DNS_FLAG_AD = 1 << 5
#DNS_FLAG_AA = 1 << 5 # authoritative answer
Copy link
Contributor

Choose a reason for hiding this comment

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

mostly curiosity... why are these commented out?

Copy link
Author

Choose a reason for hiding this comment

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

You know, that's a good q! I am not sure, they were commented out in the original version from vs1, and I didn't see a reason to change them. Tests seem to pass with or without them, but I suspect that's because they are for some case that isn't yet covered by tests.

if self.vsHasField('label'): # the resize will crash if the label hasn't been set yet w/o this
self.vsGetField('label').vsResize(size)


Copy link
Contributor

Choose a reason for hiding this comment

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

extra empty line ( which totally may have been inherited from my previous DNS cruft, but lets kill it off since we're touching it... )

Copy link
Author

Choose a reason for hiding this comment

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

killed.

self._cache_qrs = ret
return ret

def _getResourceRecords(self, structure):
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this is a top level API ( compliment of getQuestionRecords ) lets promote him by removing the _

Copy link
Author

Choose a reason for hiding this comment

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

promoted.

def __repr__(self):
return socket.inet_ntop(socket.AF_INET, bytes(self))


Copy link
Contributor

Choose a reason for hiding this comment

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

remove extra blank lines for style points :D

Copy link
Author

Choose a reason for hiding this comment

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

+1

AKOU0 added a commit to AKOU0/dissect that referenced this pull request Jul 27, 2017
@AKOU0 AKOU0 force-pushed the dns_port_to_vs2 branch from a8d9bed to fb2f92b Compare July 27, 2017 17:06
AKOU0 added a commit to AKOU0/dissect that referenced this pull request Jul 27, 2017
@AKOU0 AKOU0 force-pushed the dns_port_to_vs2 branch from fb2f92b to 9537a68 Compare July 27, 2017 17:14
@AKOU0
Copy link
Author

AKOU0 commented Jul 27, 2017

I could be wrong, but it appears that the tests are failing because the pip version of vstruct2 doesn't include the changes from PR#7 for vs2.

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