Skip to content
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.

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

fromtcpdump.t fails because Net::DNS::Packet doesn't provide TO_JSON #32

Closed
tsibley opened this issue Jan 13, 2014 · 2 comments
Closed

Comments

@tsibley
Copy link
Collaborator

tsibley commented Jan 13, 2014

JSON::XS is configured to allow blessed objects and convert them if a TO_JSON method is available. If TO_JSON is not available, however, the object is replaced by a JSON null.

I'm not sure if I should just adjust the test or if I should monkey patch TO_JSON methods into Net::DNS::Packet and related packages so that they JSON-ify as expected by RecordStream.

For example, RecordStream could do this:

sub Net::DNS::Packet::TO_JSON {
    { %{ $_[0] } }
}

although ideally it would be localized to avoid side-effects in other code:

local *Net::DNS::Packet::TO_JSON = sub {
    ....
};

Full test output below:

tom@whaam RecordStream (master=) $ prove -wlv tests/RecordStream//Operation/fromtcpdump.t 
tests/RecordStream//Operation/fromtcpdump.t .. 
ok 1 - use App::RecordStream::Operation::fromtcpdump;
ok 2 - Operation initialization
ok 3 - Record is a App::RecordStream::Record
ok 4 - Record is a App::RecordStream::Record
not ok 5 - Records match: unnamed

#   Failed test 'Records match: unnamed'
#   at /home/tom/projects/RecordStream/lib/App/RecordStream/Test/OperationHelper.pm line 104.
#     Structures begin differing at:
#          $got->[0]{dns}{count} = ARRAY(0x19b6618)
#     $expected->[0]{dns}{count} = Does not exist
ok 6 - Has called finish: unnamed
1..6
Expected and output differed!
Expected:
{"dns":{"answer":[],"buffer":"","question":[{"qclass":"IN","qname":"blog.benjaminbernard.com","qtype":"A"}],"answersize":42,"additional":[],"authority":[],"header":{"cd":0,"nscount":0,"qdcount":1,"ancount":0,"rcode":"NOERROR","tc":0,"opcode":"QUERY","ad":0,"ra":0,"qr":0,"id":3930,"arcount":0,"aa":0,"rd":1},"offset":42},"ip":{"len":70,"hlen":5,"dest_ip":"10.0.0.1","proto":17,"options":"","foffset":0,"flags":{},"ttl":64,"ver":4,"src_ip":"10.0.2.15","cksum":10544,"tos":0,"id":15208},"file":"tests/files/test-capture1.pcap","caplen":84,"length":84,"timestamp":"1294004869.88858","ethernet":{"dest_mac":"525400123502","src_mac":"080027e0fd58"},"udp":{"len":50,"src_port":46578,"cksum":5715,"dest_port":53},"type":"udp"}
{"dns":{"answer":[{"rdlength":4,"ttl":1800,"name":"blog.benjaminbernard.com","type":"A","class":"IN","address":"63.251.171.81","rdata":""},{"rdlength":4,"ttl":1800,"name":"blog.benjaminbernard.com","type":"A","class":"IN","address":"69.25.27.170","rdata":""},{"rdlength":4,"ttl":1800,"name":"blog.benjaminbernard.com","type":"A","class":"IN","address":"63.251.171.80","rdata":""},{"rdlength":4,"ttl":1800,"name":"blog.benjaminbernard.com","type":"A","class":"IN","address":"69.25.27.173","rdata":""},{"rdlength":4,"ttl":1800,"name":"blog.benjaminbernard.com","type":"A","class":"IN","address":"66.150.161.141","rdata":""},{"rdlength":4,"ttl":1800,"name":"blog.benjaminbernard.com","type":"A","class":"IN","address":"66.150.161.140","rdata":""}],"buffer":"","question":[{"qclass":"IN","qname":"blog.benjaminbernard.com","qtype":"A"}],"answersize":138,"additional":[],"authority":[],"header":{"cd":0,"nscount":0,"qdcount":1,"ancount":6,"rcode":"NOERROR","tc":0,"opcode":"QUERY","ad":0,"ra":1,"qr":1,"id":3930,"arcount":0,"aa":0,"rd":1},"offset":138},"ip":{"len":166,"hlen":5,"dest_ip":"10.0.2.15","proto":17,"options":"","foffset":0,"flags":{},"ttl":64,"ver":4,"src_ip":"10.0.0.1","cksum":23131,"tos":0,"id":2525},"file":"tests/files/test-capture1.pcap","caplen":180,"length":180,"timestamp":"1294004869.160748","ethernet":{"dest_mac":"080027e0fd58","src_mac":"525400123500"},"udp":{"len":146,"src_port":53,"cksum":47199,"dest_port":46578},"type":"udp"}
Output from module:
{"dns":null,"ip":{"hlen":5,"len":70,"proto":17,"dest_ip":"10.0.0.1","flags":{},"foffset":0,"options":"","ttl":64,"ver":4,"cksum":10544,"src_ip":"10.0.2.15","tos":0,"id":15208},"file":"tests/files/test-capture1.pcap","caplen":84,"length":84,"timestamp":"1294004869.88858","ethernet":{"src_mac":"080027e0fd58","dest_mac":"525400123502"},"udp":{"len":50,"src_port":46578,"dest_port":53,"cksum":5715},"type":"udp"}
{"dns":null,"ip":{"hlen":5,"len":166,"proto":17,"dest_ip":"10.0.2.15","flags":{},"foffset":0,"options":"","ttl":64,"ver":4,"cksum":23131,"src_ip":"10.0.0.1","tos":0,"id":2525},"file":"tests/files/test-capture1.pcap","caplen":180,"length":180,"timestamp":"1294004869.160748","ethernet":{"src_mac":"525400123500","dest_mac":"080027e0fd58"},"udp":{"len":146,"src_port":53,"dest_port":46578,"cksum":47199},"type":"udp"}
# Looks like you failed 1 test of 6.
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/6 subtests 

Test Summary Report
-------------------
tests/RecordStream//Operation/fromtcpdump.t (Wstat: 256 Tests: 6 Failed: 1)
  Failed test:  5
  Non-zero exit status: 1
Files=1, Tests=6,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.07 cusr  0.00 csys =  0.09 CPU)
Result: FAIL
@benbernard
Copy link
Owner

sigh fromtcpdump was a script I needed a couple of times back when I was doing a lot of latency analysis, I'm not sure we even need to keep it. I think this is a change in behavior from Net::DNS::Packet, because I definitely used this a bunch like 2-3 years ago. I'm cool with just skipping fromtcpdump work for now (disabling the test). If you want to fix it (would would, of course, be awesome), I'd say we should do the local monkey patch, or just post process the objects ourselves, though that can be a little crazy...

tsibley added a commit to tsibley/RecordStream that referenced this issue Jan 16, 2014
Net::DNS::Packet doesn't serialize properly yet.  JSON::XS is configured
to allow blessed objects and convert them if a TO_JSON method is
available.  If TO_JSON is not available, however, the object is replaced
by a JSON null.  Net::DNS::Packet doesn't have a TO_JSON method.
Monkey-patching one in will require futzing with a number of other
classes contained with Net::DNS::Packet objects too.

See GH benbernard#32.
@tsibley
Copy link
Collaborator Author

tsibley commented Jan 16, 2014

Hmm. Digging into the history of Net-DNS and RecordStream, I'm not sure how the test ever worked. The internal structures of Net::DNS::Packet don't match those in the expected JSON, i.e. the objects appear carefully massaged either by hand or by code that's vanished entirely.

Regardless, I've just gone and marked the test todo skip. Pull request ahoy!

@tsibley tsibley closed this as completed Feb 2, 2017
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

No branches or pull requests

2 participants