Fix: JSON-RPC param reader overshoots trailing delimiter, breaking append (#828)

[selmant]

Description

Fixes #828.

The JSON branches of XmlCommand::NextParamAsInt/Bool/Str advance m_requestPtr by + len + 1, skipping the value plus an assumed , separator. For the final element of the params array that delimiter is ], so the cursor lands past it — inside the rest of the request (e.g. ,"id":1}).

On the v13 append path the PP-Parameters loop (while (NextParamAsStr(&paramName))) then reads the request's "id" field as a parameter name and rejects the call with Invalid parameter (Parameters). The result: a JSON-RPC append with base64 content fails whenever any field follows "params" in the envelope (i.e. almost always), while it "works" if "params" happens to be last. editqueue is unaffected only because its trailing nested ]] absorbs the overshoot.

This is complementary to #702 (which fixed JsonNextValue nesting in Util.cpp); the over-advance is in the readers and was untouched by it. Still present on develop.

Fix

Advance to + len (the value's delimiter) and let JsonNextValue's existing leading-separator skip (" ,[{:\r\n\t\f") consume it on the next read. For the last element the cursor then rests on ], which the readers correctly treat as end-of-array.

This matches how the existing tests already step the helper — tests/util/Util.cpp uses JsonNextValue(keyPtr + length, ...) (no + 1). The XML-RPC and HTTP-GET branches are unchanged.

Verification

I reproduced the parser with a faithful port of JsonNextValue + the three readers and confirmed:

Behavior	append (v13, id after params)	editqueue (nested array)
current (+ len + 1)	❌ Invalid parameter (Parameters)	✅
this PR (+ len)	✅	✅

Also confirmed live against NZBGet 26.1: the same append that returned Invalid parameter (Parameters) now succeeds and returns the NZBID.

Notes for reviewers

• I don't have a local NZBGet build/test environment, so this relies on CI to compile and run the suite — please double-check.
• There's no unit-test harness for XmlCommand::NextParamAs* today (tests/remote/ doesn't exist), so I didn't add a reader-level regression test. Happy to add one (or a harness) if you'd like — guidance on the preferred location welcome.

Testing

• CI (compile + existing suite)

[dnzbk]

@selmant Hello,
Your changes indeed fix the overshoot issue for append (v13). However, advancing the pointer by + len unconditionally introduces a regression for other JSON-RPC methods like saveconfig and startscript.
In saveconfig, the parser reads an array of objects: [{"Name":"Option1","Value":"Val1"},{"Name":"Option2","Value":"Val2"}]
To fix both the original overshoot bug and avoid this regression, we should only stop (do not add 1) if the trailing character is ] or \0.
I have pushed these changes to the new fix/json-rpc branch.
I also added a dedicated RemoteTest suite with comprehensive unit tests to prevent future regressions.

I can create test builds from my branch so you can test them on your end. I’ve verified the fix for my case but can’t guarantee it will work with Bindery. Which platforms should I make builds for you?

[selmant]

Thanks @dnzbk, that makes sense. Good call adding the RemoteTest suite. I'd missed the saveconfig/startscript array-of-objects case entirely.

Linux amd64 is all I need. I'll test it against the original #828 append-with-base64 repro on my end and report back. Appreciate you putting builds together.

[dnzbk]

@selmant nzbget-linux-installer.zip