Parsing traceroute outputs without headers returns wrong results

[rarylson]

This works:

traceroute to 187.45.161.65 (187.45.161.65), 64 hops max, 52 byte packets
 1  192.168.0.1 (192.168.0.1)  1.359 ms  0.816 ms  1.408 ms
 [...] 
 5  65-161-45-187.static.in-addr.vialink.com.br (187.45.161.65)  12.930 ms  32.327 ms  10.240 ms

It returns the following Traceroute object:

Traceroute for 187.45.161.65 (187.45.161.65)

  1 192.168.0.1 (192.168.0.1) 1.359 ms
    192.168.0.1 (192.168.0.1) 0.816 ms
    192.168.0.1 (192.168.0.1) 1.408 ms

  [...]

Where self.dest_name = 187.45.161.65 and self.dest_ip = 187.45.161.65.

But when the output to parse has no headers, it doesn't work:

 1  192.168.0.1 (192.168.0.1)  1.359 ms  0.816 ms  1.408 ms
 [...]
 5  65-161-45-187.static.in-addr.vialink.com.br (187.45.161.65)  12.930 ms  32.327 ms  10.240 ms

In the current implementation, trparse parses the output, but it interprets the first line (in this case, a hop) as the header, and it returns the following Traceroute object (which is wrong):

Traceroute for 192.168.0.1 (192.168.0.1)

  1 192.168.0.1 (192.168.0.1) 1.359 ms
    192.168.0.1 (192.168.0.1) 0.816 ms
    192.168.0.1 (192.168.0.1) 1.408 ms

  [...]

Note the first line: 192.168.0.1 (192.168.0.1) is not the correct address/ip (it's the first probe name/IP instead)!

Outputs without header make sense because traceroute prints the first line (header) to stderr (instead of to stdout). So, if the user only captures stdout, the string to be parsed won't have the headers.

Some examples:

$ traceroute 187.45.161.65 >/dev/null
traceroute to 187.45.161.65 (187.45.161.65), 64 hops max, 52 byte packets
$ traceroute 187.45.161.65 2>/dev/null
 1  192.168.0.1 (192.168.0.1)  3.386 ms  1.059 ms  0.886 ms
 [...]
 5  65-161-45-187.static.in-addr.vialink.com.br (187.45.161.65)  19.414 ms  18.871 ms  10.390 ms
$ traceroute 187.45.161.65 2>&1
traceroute to 187.45.161.65 (187.45.161.65), 64 hops max, 52 byte packets
 1  192.168.0.1 (192.168.0.1)  3.386 ms  1.059 ms  0.886 ms
 [...]
 5  65-161-45-187.static.in-addr.vialink.com.br (187.45.161.65)  19.414 ms  18.871 ms  10.390 ms

I think one of the following options should be considered:

Option 1: Make explicit that we do not know the destination address / destination ip, like:

Traceroute for unknown address

  1 192.168.0.1 (192.168.0.1) 1.359 ms
  [...]

Or:

Traceroute for None (None)

  1 192.168.0.1 (192.168.0.1) 1.359 ms
  [...]

Option 2: Require the header and throw a parse error instead of wrongly parsing the output.

message = "Expected header. Got: '{}'".format(lines[0])
raise Exception(message)

[rarylson]

Hi @lbenitez000, thanks for the library (it saved me a lot of effort).

I'm working in some improvements to it. So some opened issues (and a few others I discovered while testing the lib) will be solved.

About this issue, I implemented the second solution I proposed: I'm using a stronger regexp to avoid a wrong match and, if I cannot parse the traceroute header, the parsing will fail. Another solution would be not to set the traceroute dest_name/dest_ip when header was not found.

The first solution forces the client to pass a valid traceroute output that must include the header (but it may brake code from someone that already uses your library but passes an output without header - this case is possible since the header is printed to stderr, not to stdout). In the second solution, however, passing a header is optional.

What solution do you think it's better?

PS: Soon I'll send you a pull request. Fell free to do not accept it until we agree with the better implementation to the users of your library.

[rarylson]

Hi @lbenitez000,

If you have a time, please tell me which solution makes more sense to you:

• Create the traceroute object without the destination address? (Traceroute for None (None)); or
• Require a header and raise an exception if no header is found?

I can implement one of them (in a PR) if you prefer.

Just to let you know that this issue also impacts emiliogq (issue #14).