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

Forward declarations of events don't work inside a module namespace #163

Closed
jsiwek opened this issue Sep 17, 2018 · 3 comments · Fixed by #2329
Closed

Forward declarations of events don't work inside a module namespace #163

jsiwek opened this issue Sep 17, 2018 · 3 comments · Fixed by #2329
Labels
Area: Scripting Complexity: Substantial For the stout of heart. Type: Bug 🐛 Unexpected behavior or output.

Comments

@jsiwek
Copy link
Contributor

jsiwek commented Sep 17, 2018

Moved from https://bro-tracker.atlassian.net/browse/BIT-71

Created by Robin Sommer at 2009-02-20T13:43:57.000-0600:

Forward declarations of events aren't correctly resolved when used inside a module namespace. The worst thing about this is that they fail silently: there's no error message, the handler is just not executed.

The example below never prints anything. Once the module statement is removed, everything works fine though.

module Foo;

global bar: event();

event bar()
    {
    print "bar";
    }

event new_connection(c: connection)
    {
    event bar();
    }

Comment by Jon Siwek at 2015-03-16T14:38:28.976-0500:

Other examples of the problem referenced in BIT-984.

Comment by Robin Sommer at 2016-07-08T18:46:57.988-0500:

I don't see an easy fix for this. Unfortunately event IDs are handled somewhat differently than other IDs, but the global prototype cannot adjust to that because the parser doesn't know that it's an event before it's too late.

Closing, as there's a work-around by using fully scoped event names (see BIT-984).

@jsiwek jsiwek added Complexity: Substantial For the stout of heart. Priority: High Type: Bug 🐛 Unexpected behavior or output. Area: Scripting labels Sep 17, 2018
@jsiwek
Copy link
Contributor Author

jsiwek commented Sep 27, 2018

Other pitfall examples which are sort of the reverse problem from original:

http://mailman.icsi.berkeley.edu/pipermail/bro-dev/2018-September/013057.html

So assuming the main source of confusion is that event declarations implicitly add module prefixes to names, an idea would be to try to make event handler names also implicitly add module prefixes when they are not ambiguous with a name already in the global namespace. The name resolution process for things like event calling sites may also need adjusting -- e.g. if no ambiguous event declaration in the global namespace, then instead raise an event prefixed by current module name.

@timwoj
Copy link
Member

timwoj commented Jul 2, 2022

At least we're now getting a warning for the test case in the first post, thanks to Vern's recent work:

warning in ./test.zeek, line 3: handler for non-existing event cannot be invoked (Foo::bar)

@vpax
Copy link
Contributor

vpax commented Jul 2, 2022

Good to hear, as that was in fact exactly the use case (which I ran across in a different context) that made me think "geez we need to find these"!

timwoj added a commit that referenced this issue Aug 17, 2022
…possible

The change to the capture-loss test is actually a fix for a bug exposed by the
code change. Previously it wasn't firing the scheduled event because of a failed
name lookup. Now that the lookup has been fixed, the event happens twice.
timwoj added a commit that referenced this issue Aug 17, 2022
* origin/topic/timw/163-event-lookup:
  GH-163: Use ID name (including module name) to create EventExpr when possible
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Scripting Complexity: Substantial For the stout of heart. Type: Bug 🐛 Unexpected behavior or output.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants