From fccb9ccab035a9add7b7dee9d26398a761c1b2e0 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Thu, 14 Apr 2022 09:54:45 +0200 Subject: [PATCH] Re-instantiate providing location information to `LoadFile` hooks. #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. --- src/Obj.h | 1 + src/RuleMatcher.cc | 10 +++++++ src/ScannedFile.cc | 5 ++-- src/ScannedFile.h | 4 ++- src/scan.l | 7 ++++- .../plugins.plugin-load-file-extended/output | 8 +++--- .../plugins/plugin-load-file-extended.zeek | 16 +++++++++++- .../plugin-load-file-extended/src/Plugin.cc | 26 ++++++++++++++++--- 8 files changed, 66 insertions(+), 11 deletions(-) diff --git a/src/Obj.h b/src/Obj.h index 0926e23512..a62ca4886a 100644 --- a/src/Obj.h +++ b/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("", 0, 0, 0, 0); diff --git a/src/RuleMatcher.cc b/src/RuleMatcher.cc index 42c3c568b0..a418c30cc6 100644 --- a/src/RuleMatcher.cc +++ b/src/RuleMatcher.cc @@ -266,6 +266,13 @@ bool RuleMatcher::ReadFiles(const std::vector& 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> 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& 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: diff --git a/src/ScannedFile.cc b/src/ScannedFile.cc index 590511050e..0e25b47a27 100644 --- a/src/ScannedFile.cc +++ b/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) - : file(std::move(file)), full_path(std::move(full_path)) +SignatureFile::SignatureFile(std::string file, std::string full_path, Location load_location) + : file(std::move(file)), full_path(std::move(full_path)), + load_location(std::move(load_location)) { } diff --git a/src/ScannedFile.h b/src/ScannedFile.h index 9829b2c1d2..b51369ef8f 100644 --- a/src/ScannedFile.h +++ b/src/ScannedFile.h @@ -2,6 +2,7 @@ #pragma once +#include #include #include #include @@ -40,9 +41,10 @@ struct SignatureFile { std::string file; std::optional 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 sig_files; diff --git a/src/scan.l b/src/scan.l index a53c247798..52d2896692 100644 --- a/src/scan.l +++ b/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); diff --git a/testing/btest/Baseline/plugins.plugin-load-file-extended/output b/testing/btest/Baseline/plugins.plugin-load-file-extended/output index d32f2141d1..3d5b86d6dd 100644 --- a/testing/btest/Baseline/plugins.plugin-load-file-extended/output +++ b/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=|yyy| resolved=|| -HookLoadExtended/signature: file=|abc.sig| resolved=|./abc.sig| +HookLoadExtended/script: file=|xxx| resolved=|./xxx.zeek| srcloc=|n/a| +HookLoadExtended/script: file=|xxx3| resolved=|./xxx3.zeek| srcloc=|./xxx2.zeek| +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! diff --git a/testing/btest/plugins/plugin-load-file-extended.zeek b/testing/btest/plugins/plugin-load-file-extended.zeek index 0d3d297c5a..2550363fd3 100644 --- a/testing/btest/plugins/plugin-load-file-extended.zeek +++ b/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 diff --git a/testing/btest/plugins/plugin-load-file-extended/src/Plugin.cc b/testing/btest/plugins/plugin-load-file-extended/src/Plugin.cc index a86d4f50f2..2785b30c9e 100644 --- a/testing/btest/plugins/plugin-load-file-extended/src/Plugin.cc +++ b/testing/btest/plugins/plugin-load-file-extended/src/Plugin.cc @@ -26,9 +26,15 @@ std::pair> 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> 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> 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> 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); }