Restore single-argument Json.decode() in the DSL

[eimann]

Please note: I based this against support/2.16 as the security fix isn't on master yet (also explained below). Also this is my first contribution to the Icinga code base. But as we are a heavy Icinga user and discovered the bug I took the chance.

Since 2.16.2, Json.decode("...") fails with Too few arguments for function, an undocumented breaking change in a patch release (reported in #10913).

Cause

The recursion depth limit added to JsonDecode() in 2.16.2 (ed163a8) gave the C++ function a second parameter with a default value:

Value JsonDecode(const String& data, size_t depthLimit = JsonDecodeDefaultDepthLimit);

The DSL binding in json-script.cpp passed this function pointer directly to Function. Required-argument count is deduced from the function-pointer arity via boost::function_types::function_arity, and function pointers do not carry default arguments — so the arity became 2 and Json.decode() started requiring two arguments.

Fix

Wrap JsonDecode() in a single-argument shim (mirroring the existing JsonEncodeShim) so the registered function keeps its one-parameter contract while still applying the default depth limit internally. The 2.16.2 security hardening is fully preserved.

Adds a regression test that invokes the registered Json.decode function with a single argument (it fails against the unfixed binding with the exact Too few arguments error).

Notes

• Targets support/2.16: the regression exists only there. master never received the depth-limit change, so it is unaffected. When the depth limit is forward-ported to master, this shim should accompany it.

fixes #10913

[cla-bot]

Thank you for your pull request. Before we can look at it, you'll need to sign a Contributor License Agreement (CLA).

Please follow instructions at https://icinga.com/company/contributor-agreement to sign the CLA.

After that, please reply here with a comment and we'll verify.

Contributors that have not signed yet: @eimann

Details

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Please contact us if you think this is the case.

• If you signed the CLA as a corporation, your GitHub username may not have been submitted to us. Please reach out to the responsible person in your organization.

[eimann]

CLA is now signed.

[bobapple]

@cla-bot check

[julianbrost]

Ugh, that's annoying, there always has to be something that everyone misses... :(

Thanks for looking into this!

• Targets support/2.16: the regression exists only there. master never received the depth-limit change, so it is unaffected. When the depth limit is forward-ported to master, this shim should accompany it.

It now is with #10908, it's just that we did the changes to the master branch as regular GitHub PRs, so they took a bit longer to merge.

[julianbrost]

@eimann Judging from that emoji reaction, you're reading these messages in a timely manner. At first sight, your changes look good, would you mind just rebasing them onto the master and changing the target branch of the PR accordingly? Or just let us know if you don't have time for this right now, then we'd just take over from here as we'll probably want to get this fix out rather quickly.

[eimann]

Gimme a sec, I'll get it done :)

[eimann]

Should be good now. I also deployed 2.16.2 with the fix on our test system, loading the config fine. No more errors on icinga console.

[julianbrost]

Hint for anyone who will have a look at the backported PRs: even if they are created without conflicts, the older support branches don't include the refactoring that auto-detects test cases and needs them to be added to CMakeLists additionally.

[julianbrost]

Oh come on GitHub, why are you showing the Actions checks as "Expected", not "Queued" (as it's shown in other PRs where the jobs wait for a slot)? Looks like we can wait a long time for them to run then...

Don't be confused that I'll close the PR. I'll reopen it immediately, maybe that helps GitHub to get things going.

[eimann]

Unfortunately this seems to be a GitHub issue since ~ 45-60 Minutes, we see it on private repos as well but github status is showing all green :(

[Al2Klimov]

Ah, yes, the good old close/open PR trick @yhabteab told me not to use for GHA))

[julianbrost]

Closing and re-opening the PR made the "approve workflow runs" button appear again. After clicking that one, they are running now. So looks like we're good now.

[eimann]

Looks like choco install -y winflexbison3 got a 503 for the Win32 build so it failed

[Al2Klimov]

We can retry it only once Windows completes.

[backbot-ci]

Successfully created backport PR for support/2.15:

• [Backport support/2.15] Restore single-argument Json.decode() in the DSL #10920

[backbot-ci]

Backport failed for support/2.14, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin support/2.14
git worktree add -d .worktree/backport-10914-to-support/2.14 origin/support/2.14
cd .worktree/backport-10914-to-support/2.14
git switch --create backport-10914-to-support/2.14
git cherry-pick -x d29ac491f231d6831e98cc614902faed78718598 326cbde4476f5d5a5695dca5ef8b2492c0280053

[backbot-ci]

Successfully created backport PR for support/2.16:

• [Backport support/2.16] Restore single-argument Json.decode() in the DSL #10921

[julianbrost]

Backport failed for support/2.14, because it was unable to cherry-pick the commit(s).

I'll take care of that one.