Merge remote-tracking branch 'origin/topic/robin/loads-sigs-path'

* origin/topic/robin/loads-sigs-path:
  Re-instantiate providing location information to `LoadFile` hooks.

CHANGES

@@ -1,3 +1,13 @@
+5.0.0-dev.257 | 2022-04-14 10:13:28 -0700
+
+  * Re-instantiate providing location information to `LoadFile` hooks. (Robin Sommer, Corelight)
+
+    #1835 subtly changed the semantics of the `LoadFile` plugin hook to no
+    longer have the current script location available for signature files
+    being loaded through `@load-sigs`. This was undocumented behavior, so
+    it's technically not a regression, but since at least one external
+    plugin is depending on it, this change restores the old behavior.
+
 5.0.0-dev.255 | 2022-04-14 10:12:49 -0700
 
   * Fix another crash during dictionary iteration. (Robin Sommer, Corelight)

VERSION

@@ -1 +1 @@
-5.0.0-dev.255
+5.0.0-dev.257

src/Obj.h

@@ -38,6 +38,7 @@ public:
 #define YYLTYPE zeek::detail::yyltype
 using yyltype = Location;
 YYLTYPE GetCurrentLocation();
+void SetCurrentLocation(YYLTYPE currloc);
 
 // Used to mean "no location associated with this object".
 inline constexpr Location no_location("<no location>", 0, 0, 0, 0);

src/RuleMatcher.cc

@@ -266,6 +266,13 @@ bool RuleMatcher::ReadFiles(const std::vector<SignatureFile>& files)
 		if ( ! f.full_path )
 			f.full_path = util::find_file(f.file, util::zeek_path(), ".sig");
 
+		// We mimic previous Zeek versions by temporarily setting the current
+		// script location to the place where the loading happened. This
+		// behavior was never documented, but seems worth not breaking as some
+		// plugins ended up relying on it.
+		Location orig_location = detail::GetCurrentLocation();
+		detail::SetCurrentLocation(f.load_location);
+
 		std::pair<int, std::optional<std::string>> rc = {-1, std::nullopt};
 		rc.first = PLUGIN_HOOK_WITH_RESULT(
 			HOOK_LOAD_FILE, HookLoadFile(zeek::plugin::Plugin::SIGNATURES, f.file, *f.full_path),
@@ -277,6 +284,9 @@ bool RuleMatcher::ReadFiles(const std::vector<SignatureFile>& files)
 				HookLoadFileExtended(zeek::plugin::Plugin::SIGNATURES, f.file, *f.full_path),
 				std::make_pair(-1, std::nullopt));
 
+		// Restore original location information.
+		detail::SetCurrentLocation(orig_location);
+
 		switch ( rc.first )
 			{
 			case -1:

src/ScannedFile.cc

@@ -49,8 +49,9 @@ bool ScannedFile::AlreadyScanned() const
 
 SignatureFile::SignatureFile(std::string file) : file(std::move(file)) { }
 
-SignatureFile::SignatureFile(std::string file, std::string full_path)
+SignatureFile::SignatureFile(std::string file, std::string full_path, Location load_location)
-	: file(std::move(file)), full_path(std::move(full_path))
+	: file(std::move(file)), full_path(std::move(full_path)),
+	  load_location(std::move(load_location))
 	{
 	}
 

src/ScannedFile.h

@@ -2,6 +2,7 @@
 
 #pragma once
 
+#include <zeek/Obj.h>
 #include <list>
 #include <optional>
 #include <string>
@@ -40,9 +41,10 @@ struct SignatureFile
 	{
 	std::string file;
 	std::optional<std::string> full_path;
+	Location load_location;
 
 	SignatureFile(std::string file);
-	SignatureFile(std::string file, std::string full_path);
+	SignatureFile(std::string file, std::string full_path, Location load_location);
 	};
 
 extern std::vector<SignatureFile> sig_files;

src/scan.l

@@ -374,7 +374,7 @@ when	return TOK_WHEN;
 @load-sigs{WS}{FILE} {
 	const char* file = zeek::util::skip_whitespace(yytext + 10);
 	std::string path = find_relative_file(file, ".sig");
-	sig_files.emplace_back(file, path);
+	sig_files.emplace_back(file, path, GetCurrentLocation());
 	}
 
 @load-plugin{WS}{ID} {
@@ -586,6 +586,11 @@ YYLTYPE zeek::detail::GetCurrentLocation()
 	return currloc;
 	}
 
+void zeek::detail::SetCurrentLocation(YYLTYPE currloc) {
+	::filename = currloc.filename;
+	line_number = currloc.first_line;
+}
+
 static int load_files(const char* orig_file)
 	{
 	std::string file_path = find_relative_script_file(orig_file);

testing/btest/Baseline/plugins.plugin-load-file-extended/output

@@ -1,7 +1,9 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-HookLoadExtended/script: file=|xxx| resolved=|./xxx.zeek|
+HookLoadExtended/script: file=|xxx| resolved=|./xxx.zeek| srcloc=|n/a|
-HookLoadExtended/script: file=|yyy| resolved=||
+HookLoadExtended/script: file=|xxx3| resolved=|./xxx3.zeek| srcloc=|./xxx2.zeek|
-HookLoadExtended/signature: file=|abc.sig| resolved=|./abc.sig|
+HookLoadExtended/script: file=|yyy| resolved=|| srcloc=|n/a|
+HookLoadExtended/signature: file=|abc.sig| resolved=|./abc.sig| srcloc=|n/a|
+HookLoadExtended/signature: file=|def.sig| resolved=|./def.sig| srcloc=|./xxx2.zeek|
 new zeek_init(): script has been replaced
 new zeek_init(): script has been added
 signature works!

testing/btest/plugins/plugin-load-file-extended.zeek

@@ -1,7 +1,7 @@
 # @TEST-EXEC: ${DIST}/auxil/zeek-aux/plugin-support/init-plugin -u . Testing LoadFileExtended
 # @TEST-EXEC: cp -r %DIR/plugin-load-file-extended/* .
 # @TEST-EXEC: ./configure --zeek-dist=${DIST} && make
-# @TEST-EXEC: ZEEK_PLUGIN_PATH=$(pwd) zeek -r $TRACES/wikipedia.trace -b Testing::LoadFileExtended xxx yyy -s abc.sig >> output
+# @TEST-EXEC: ZEEK_PLUGIN_PATH=$(pwd) zeek -r $TRACES/wikipedia.trace -b Testing::LoadFileExtended xxx xxx2 yyy -s abc.sig >> output
 # @TEST-EXEC: btest-diff output
 
 # @TEST-START-FILE xxx.zeek
@@ -12,6 +12,20 @@ event zeek_init() {
 
 # @TEST-END-FILE
 
+# @TEST-START-FILE xxx2.zeek
+# Test loading from script land.
+@load xxx3
+@load-sigs def.sig
+# @TEST-END-FILE
+
+# @TEST-START-FILE xxx3.zeek
+# empty
+# @TEST-END-FILE
+
 # @TEST-START-FILE abc.sig
 # empty
 # @TEST-END-FILE
+
+# @TEST-START-FILE def.sig
+# empty
+# @TEST-END-FILE

testing/btest/plugins/plugin-load-file-extended/src/Plugin.cc

@@ -26,9 +26,15 @@ std::pair<int, std::optional<std::string>> Plugin::HookLoadFileExtended(const Lo
                                                                         const std::string& file,
                                                                         const std::string& resolved)
 	{
+	// Zeek implicitly provides the location where the current '@load'
+	// originated. If no location is available, filename will be a nullptr.
+	auto src = zeek::detail::GetCurrentLocation().filename;
+	if ( ! src )
+		src = "n/a";
+
 	if ( type == LoadType::SCRIPT && file == "xxx" )
 		{
-		printf("HookLoadExtended/script: file=|%s| resolved=|%s|\n", file.c_str(), resolved.c_str());
+		printf("HookLoadExtended/script: file=|%s| resolved=|%s| srcloc=|%s|\n", file.c_str(), resolved.c_str(), src);
 
 		return std::make_pair(1, R"(
 			event zeek_init() {
@@ -41,9 +47,16 @@ std::pair<int, std::optional<std::string>> Plugin::HookLoadFileExtended(const Lo
 		)");
 		}
 
+	if ( type == LoadType::SCRIPT && file == "xxx3" )
+		{
+		printf("HookLoadExtended/script: file=|%s| resolved=|%s| srcloc=|%s|\n", file.c_str(), resolved.c_str(), src);
+		// We don't replace this one.
+		return std::make_pair(-1, std::nullopt);
+		}
+
 	if ( type == LoadType::SCRIPT && file == "yyy" )
 		{
-		printf("HookLoadExtended/script: file=|%s| resolved=|%s|\n", file.c_str(), resolved.c_str());
+		printf("HookLoadExtended/script: file=|%s| resolved=|%s| srcloc=|%s|\n", file.c_str(), resolved.c_str(), src);
 
 		return std::make_pair(1, R"(
 			event zeek_init() {
@@ -54,7 +67,7 @@ std::pair<int, std::optional<std::string>> Plugin::HookLoadFileExtended(const Lo
 
 	if ( type == LoadType::SIGNATURES && file == "abc.sig" )
 		{
-		printf("HookLoadExtended/signature: file=|%s| resolved=|%s|\n", file.c_str(), resolved.c_str());
+		printf("HookLoadExtended/signature: file=|%s| resolved=|%s| srcloc=|%s|\n", file.c_str(), resolved.c_str(), src);
 
 		return std::make_pair(1, R"(
 		signature my-sig {
@@ -65,6 +78,13 @@ std::pair<int, std::optional<std::string>> Plugin::HookLoadFileExtended(const Lo
 		)");
 		}
 
+	if ( type == LoadType::SIGNATURES && file == "def.sig" )
+		{
+		printf("HookLoadExtended/signature: file=|%s| resolved=|%s| srcloc=|%s|\n", file.c_str(), resolved.c_str(), src);
+		// We don't replace this one.
+		return std::make_pair(-1, std::nullopt);
+		}
+
 	return std::make_pair(-1, std::nullopt);
 	}