%init hook of context units not invoked

[awelzel]

When using a unit type for %context, the unit's %init hook is not invoked.

Ran into it when attempting to instantiate an analyzer from within a context's %init to have an easy place where this can be done once for a connection (not anymore sure this is actually a good approach, but the non-running %init is still surprising).

module ParseBytes;

import zeek;

type MyContext = unit {

  var counter: uint64 = 1;
  var secret: bytes;

  on %init {
    # None of the below is running.
    print "never seen";

    self.secret = b"super secret";
    self.counter = 0;  # actually, start with 0, not 1.

    # Always add SSL child-analyzer.
    zeek::protocol_begin("SSL");
  }
};

public type OneByte = unit(ctx: MyContext) {
    b: bytes &size=1 {
      ++ctx.counter;
      # Try feeding to SSL.
      zeek::protocol_data_in(zeek::is_orig(), $$);
    }

    on %done {
      print "ctx", ctx, "b", self.b;
    }
};

public type ManyBytes = unit {
    %context = MyContext;
    : OneByte(self.context()) [];
};

I can see an argument to not support the hooks and treat the context units as purely data holding store. In that case maybe should hard-reject %context = UnitWithHooks.

[bbannier]

I think we had a few instances already where users attempted to use a unit-type as context. Units come with additional code for parsing and hooks which are execute during it. In most such situations (also here) a struct type is what you want. Structs do not support e.g., %init hooks.

[awelzel]

Units come with additional code for parsing and hooks which are execute during it.

Ah, that clears the confusion - %init and %done are specific to parsing and not so much constructor / destructor methods.

Mainly spurred by this snippet that I happily tried to extend.

[bbannier]

Mainly spurred by this snippet that I happily tried to extend.

Yes, this definitely should use a struct instead of a unit. This specific snippet seems to initialize all context fields to default values of their respective type, so I do not think any explicit initialization is needed here.

In general, with non-standard default values for struct fields they can be less convenient than units since we do not support any constructor syntax for them, and field values cannot be declared with a default value inline. Instead their values need to be explicitly initialized everywhere the context value is created.

[rsmmr]

in general, with non-standard default values for struct fields they can be less convenient than units since we do not support any constructor syntax for them,

Actually, we do:

type X = struct {
    a: uint8;
    b: uint8;
};

function f() {
    local x: X = [$a = 1, $b = 2];
    print x;
}

[bbannier]

No custom default constructor syntax ;)

[awelzel]

I'm closing as I'm not sure there's anything reasonable to do here. Maybe don't allow units as %context could be one?

There were two independent cases where %init was implemented to initialize vars of units only used only as a context, but maybe these can be caught review or a linter.

[0xxon]

I came across this due to https://github.com/zeek/zeek/pull/3765.

I think that this either should be supported, or raise a compilation error. Just silently not executing %init seems very unexpected to me.

[bbannier]

I think that this either should be supported, or raise a compilation error. Just silently not executing %init seems very unexpected to me.

For custom data to pass as contexts struct (one collection of named fields) is preferred over unit (can parse). While unusual I do not think it is completely out of question that somebody might want to pass a unit in a context (e.g., imagine using parsed data as information in context); for that reason I think we wouldn't want to outright disallow this.

The documentation of %init states:

on %init() { ... }:

Executes just before unit parsing will start.

In your case we never started parsing so %init was not called.