Behavior changes due to deferring record field initialization

[awelzel]

Creating as a discussion as maybe this will never come up in practice, but I'm suspecting this is bug/regression material or minimally new subtle gotchas one needs to be aware of.

With deferred initialization of aggregate record fields, any nested record fields using non-const default values (network_time() / current_time(), ...) are now evaluated at access time only.

Below script is somewhat constructed in that it uses &default heavily, but does demonstrate the behavior change due to deferred initialization of Details until connection_state_remove(). At that point &default expressions yield completely different values as compared to new_connection().

Concretely, the ts values of the outer ConnInfo and the inner Details disagree with 6.0.0, while they are the same with Zeek 5.2.2. The other is that get_current_packet_header() is evaluated during connection_state_remove() so it's not first_pkt_hdr and Zeek doesn't provide a packet at that time anymore either.

# x.zeek
type Details: record {
  ts: time &default=network_time();
  first_pkt_hdr: raw_pkt_hdr &default=get_current_packet_header();
};

type ConnInfo: record {
  ts: time &default=network_time();
  d: Details;
};

global conns: table[string] of ConnInfo;

event new_connection(c: connection) {
  conns[c$uid] = ConnInfo();
}

event connection_state_remove(c: connection) {
  print conns[c$uid];
  delete conn[c$uid];
}

# Zeek 5.2.2
$ /opt/zeek-5.2.2/bin/zeek -r ../../testing/btest/Traces/http/get.trace x.zeek 
[ts=1362692526.869344, d=[ts=1362692526.869344, first_pkt_hdr=[l2=[encap=LINK_ETHERNET, len=78, cap_len=78, src=c8:bc:c8:96:d2:a0, dst=00:10:db:88:d2:ef, vlan=<uninitialized>, inner_vlan=<uninitialized>, eth_type=2048, proto=L3_IPV4], ip=[hl=20, tos=0, len=64, id=57230, ttl=64, p=6, src=141.142.228.5, dst=192.150.187.43], ip6=<uninitialized>, tcp=[sport=59856/tcp, dport=80/tcp, seq=4263588410, ack=0, hl=44, dl=0, reserved=0, flags=2, win=65535], udp=<uninitialized>, icmp=<uninitialized>]]]

# Zeek 6.0
$ /opt/zeek-6.0-release/bin/zeek -r ../../testing/btest/Traces/http/get.trace x.zeek 
[ts=1362692526.869344, d=[ts=1362692527.080972, first_pkt_hdr=[l2=[encap=<uninitialized>, len=<uninitialized>, cap_len=<uninitialized>, src=<uninitialized>, dst=<uninitialized>, vlan=<uninitialized>, inner_vlan=<uninitialized>, eth_type=<uninitialized>, proto=<uninitialized>], ip=<uninitialized>, ip6=<uninitialized>, tcp=<uninitialized>, udp=<uninitialized>, icmp=<uninitialized>]]]

I'm suspecting a possible fix is to not defer initialization of aggregate records if they recursively contain non-const default expressions on any of their fields.

[0xxon]

I agree that this is not nice, makes it easy to shoot yourself into the foot in really hard to debug ways - and I like your idea of a fix for this.

[awelzel]

This has been fixed.