On 11/22/19 6:21 PM, Stoiko Ivanov wrote:
> When validating the parameters (or return values) of an API call against the
> schema we run the parameter hash through Devel::Cycle::find_cycle.
>
> find_cycle has no handler for GLOB types (e.g. filehandles).
> Recently we introduced passing a filehandle as return attribute in
> PMG::API2::Quarantine::download
>
> When find_cycle encounters such an object it emits a warning:
> `Unhandled type: GLOB at /usr/share/perl5/Devel/Cycle.pm line 109`
> which ends up in the journal of pmgproxy (in that example).
>
> Since the line might mislead someone to think that this is a more serious bug
> we silence it, by overriding the warnhandler ($SIG{'__WARN__'}, [0,1]) for the
> time when find_cycle runs, to ignore messages starting with
> 'Unhandled type: GLOB at'. The particular location of and in in Devel::Cycle
> might change and is left out of the match.
>
> [0] man perlvar
> [1] perldoc -f warn
>
> Signed-off-by: Stoiko Ivanov <s.iva...@proxmox.com>
> ---
>
> sending as RFC, since the comment above the patch already indicate that we
> could do away with the cycle-search on every call
I'd really rather to that, this is a build check, and thus we
do not want to do it on every validation - besides those errors
you ran into, it's also a bit costly..
I'd check for an environment variable, e.g. for "DEB_BUILD_ARCH",
which we always have set when building. As some may want to have
validation also in other cases we could also check a opt-in one,
for example:
return if $ENV{DEB_BUILD_ARCH} && !$ENV{PROXMOX_FORCE_SCHEMA_VALIDATION};
>
> The choice of saving the old handler and restoring it instead of using
> local $SIG{'__WARN__'} was done to minimize the effect - although local'izing
> is a bit more idiomatic it seems.
>
> Caught this in my PMG's journal - traced it down, by editing
> /usr/share/perl5/Devel/Cycle and adding a Carp::cluck at the location the
> warning came from)
>
> at /usr/share/perl5/Devel/Cycle.pm line 108.
> Devel::Cycle::_find_cycle_dispatch(IO::File=GLOB(0x559b8b002420),
> HASH(0x559b8ad928b8), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40),
> ARRAY(0x559b8acfd088)) called at /usr/share/perl5/Devel/Cycle.pm line 97
> Devel::Cycle::_find_cycle(IO::File=GLOB(0x559b8b002420),
> HASH(0x559b8b0b6c80), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40),
> ARRAY(0x559b8acfd088)) called at /usr/share/perl5/Devel/Cycle.pm line 156
> Devel::Cycle::_find_cycle_HASH(HASH(0x559b8b002450),
> HASH(0x559b8ad8d088), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40)) called
> at /usr/share/perl5/Devel/Cycle.pm line 114
> Devel::Cycle::_find_cycle_dispatch(HASH(0x559b8b002450),
> HASH(0x559b8ad8d088), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40)) called
> at /usr/share/perl5/Devel/Cycle.pm line 97
> Devel::Cycle::_find_cycle(HASH(0x559b8b002450), HASH(0x559b87def760),
> CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40)) called at
> /usr/share/perl5/Devel/Cycle.pm line 70
> Devel::Cycle::find_cycle(HASH(0x559b8b002450), CODE(0x559b8acfe4f0))
> called at /usr/share/perl5/PVE/JSONSchema.pm line 1053
> PVE::JSONSchema::validate(HASH(0x559b8b002450), HASH(0x559b8ab6d330),
> "Result verification failed\x{a}") called at
> /usr/share/perl5/PVE/RESTHandler.pm line 449
> PVE::RESTHandler::handle("PMG::API2::Quarantine",
> HASH(0x559b8ab6d738), HASH(0x559b8ad23c10)) called at
> /usr/share/perl5/PMG/HTTPServer.pm line 157
> eval {...} called at /usr/share/perl5/PMG/HTTPServer.pm line 111
> PMG::HTTPServer::rest_handler(PMG::HTTPServer=HASH(0x559b8ac60ad8),
> "192.168.16.68", "GET", "/quarantine/download", HASH(0x559b8acc0300),
> HASH(0x559b8706b9c8), "json") called at
> /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 725
> eval {...} called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line
> 699
>
> PVE::APIServer::AnyEvent::handle_api2_request(PMG::HTTPServer=HASH(0x559b8ac60ad8),
> HASH(0x559b8708f268), HASH(0x559b8acc0300), "GET",
> "/api2/json/quarantine/download") called at
> /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 950
> eval {...} called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line
> 942
>
> PVE::APIServer::AnyEvent::handle_request(PMG::HTTPServer=HASH(0x559b8ac60ad8),
> HASH(0x559b8708f268), HASH(0x559b8acc0300), "GET",
> "/api2/json/quarantine/download") called at
> /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 1342
> eval {...} called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line
> 1160
>
> PVE::APIServer::AnyEvent::__ANON__(AnyEvent::Handle=HASH(0x559b87057838), "",
> "\x{d}\x{a}") called at
> /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Handle.pm line 1541
> AnyEvent::Handle::__ANON__(AnyEvent::Handle=HASH(0x559b87057838))
> called at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Handle.pm line 1315
> AnyEvent::Handle::_drain_rbuf(AnyEvent::Handle=HASH(0x559b87057838))
> called at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Handle.pm line 2100
> AnyEvent::Handle::_dotls(AnyEvent::Handle=HASH(0x559b87057838))
> called at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Handle.pm line 2013
> AnyEvent::Handle::__ANON__(EV::IO=SCALAR(0x559b8acbfcd0), 1) called
> at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Impl/EV.pm line 88
> eval {...} called at
> /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Impl/EV.pm line 88
>
> AnyEvent::CondVar::Base::_wait(AnyEvent::CondVar=HASH(0x559b8708f8b0)) called
> at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent.pm line 2026
> AnyEvent::CondVar::Base::recv(AnyEvent::CondVar=HASH(0x559b8708f8b0))
> called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 1623
> PVE::APIServer::AnyEvent::run(PMG::HTTPServer=HASH(0x559b8ac60ad8))
> called at /usr/share/perl5/PMG/Service/pmgproxy.pm line 132
>
> PMG::Service::pmgproxy::run(PMG::Service::pmgproxy=HASH(0x559b8ac1ca00))
> called at /usr/share/perl5/PVE/Daemon.pm line 171
> eval {...} called at /usr/share/perl5/PVE/Daemon.pm line 171
> PVE::Daemon::__ANON__(PMG::Service::pmgproxy=HASH(0x559b8ac1ca00))
> called at /usr/share/perl5/PVE/Daemon.pm line 391
> eval {...} called at /usr/share/perl5/PVE/Daemon.pm line 380
> PVE::Daemon::__ANON__(PMG::Service::pmgproxy=HASH(0x559b8ac1ca00), 1)
> called at /usr/share/perl5/PVE/Daemon.pm line 552
> eval {...} called at /usr/share/perl5/PVE/Daemon.pm line 550
> PVE::Daemon::start(PMG::Service::pmgproxy=HASH(0x559b8ac1ca00), 1)
> called at /usr/share/perl5/PVE/Daemon.pm line 660
> PVE::Daemon::__ANON__(HASH(0x559b8543fbd8)) called at
> /usr/share/perl5/PVE/RESTHandler.pm line 446
> PVE::RESTHandler::handle("PMG::Service::pmgproxy",
> HASH(0x559b8ac4bf68), HASH(0x559b8543fbd8)) called at
> /usr/share/perl5/PVE/RESTHandler.pm line 855
> eval {...} called at /usr/share/perl5/PVE/RESTHandler.pm line 838
> PVE::RESTHandler::cli_handler("PMG::Service::pmgproxy", "pmgproxy
> start", "start", ARRAY(0x559b8574f330), ARRAY(0x559b8ac55bc0), undef, undef,
> undef) called at /usr/share/perl5/PVE/CLIHandler.pm line 590
> PVE::CLIHandler::__ANON__(ARRAY(0x559b8543ff38),
> CODE(0x559b85489128), undef) called at /usr/share/perl5/PVE/CLIHandler.pm
> line 667
> PVE::CLIHandler::run_cli_handler("PMG::Service::pmgproxy", "prepare",
> CODE(0x559b85489128)) called at /usr/bin/pmgproxy line 31
>
>
> src/PVE/JSONSchema.pm | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
> index 51c3881..6954241 100644
> --- a/src/PVE/JSONSchema.pm
> +++ b/src/PVE/JSONSchema.pm
> @@ -1038,8 +1038,22 @@ sub validate {
> # todo: cycle detection is only needed for debugging, I guess
> # we can disable that in the final release
> # todo: is there a better/faster way to detect cycles?
> +
> + # override warn handler to suppress log-messages for e.g. File::IO
> objects
> + # in the API definitions (e.g. PMG::API2::Quarantine::download)
> + my $oldwarn = $SIG{'__WARN__'};
> + $SIG{'__WARN__'} = sub {
> + my ($warn) = @_;
> + if ($warn !~ /^Unhandled type: GLOB at/) {
> + print STDERR "$warn\n";
> + }
> + };
> +
> my $cycles = 0;
> find_cycle($instance, sub { $cycles = 1 });
> +
> + $SIG{'__WARN__'} = $oldwarn;
> +
> if ($cycles) {
> add_error($errors, undef, "data structure contains recursive cycles");
> } elsif ($schema) {
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
