Reject unrecognised directoryType in jamulusserver/setDirectory

[mcfnord]

🤖 AI: Short description of changes

jamulusserver/setDirectory replied "ok" to any directoryType string and mapped an unrecognised one to any_genre_1, the public default, because DeserializeDirectoryType() returned AT_DEFAULT on a lookup miss. The same miss made the existing "custom needs an address" guard unreachable for "CUSTOM". The lookup now returns bool with an out-parameter; an unrecognised value returns error -32602 with the message pljones asked for on #3915, and the directory setting is left unchanged. The doc comment no longer claims the result is always "ok", and docs/JSON-RPC.md is regenerated.

CHANGELOG: Server: jamulusserver/setDirectory rejects an unrecognised directoryType instead of silently registering the server with the default public directory.

Context: Fixes an issue?

Fixes: #3915

Does this change need documentation? What needs to be documented and how?

docs/JSON-RPC.md is regenerated in this PR. Nothing on the website.

Status of this Pull Request

Working implementation.

What is missing until this pull request can be merged?

Review. Tested on loopback against the patched server: eight unrecognised values (NONE, CUSTOM, custon, custom with a trailing space, GENRE_ROCK, genre-rock, "", unknown_xyz) all return -32602 and getServerProfile still reports the previous type; none still applies (the other valid values were unchanged by this revision and were verified in the earlier 24-case A/B); custom without an address is still rejected by the existing guard. Builds clean with -Wall -Wextra; clang-format clean.

Checklist

• I've verified that this Pull Request follows the general code principles
• I tested my code and it does what I want
• My code follows the style guide
• I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
• I've filled all the content above

---

🤖 This message was written by AI and reviewed by @mcfnord.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: QUIET

Plan: Advanced

Run ID: f59083b2-458e-40d7-9b74-41961794bbfb

📥 Commits

Reviewing files that changed from the base of the PR and between 43718fd and b7818c7.

📒 Files selected for processing (3)

• docs/JSON-RPC.md
• src/serverrpc.cpp
• src/serverrpc.h

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

setDirectory now rejects unrecognized directoryType values with error -32602 and leaves the directory setting unchanged. The lookup reports success through a boolean result and output parameter. The RPC documentation describes accepted values and exact matching rules.

Changes

Directory type validation

Layer / File(s)	Summary
Validate setDirectory input src/serverrpc.h, src/serverrpc.cpp, docs/JSON-RPC.md	DeserializeDirectoryType now reports whether the input matches an accepted value. setDirectory returns error -32602 for unrecognized values without changing the directory setting. The documentation lists accepted values and states that matching is case-sensitive and does not trim whitespace.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: dingodoppelt

Merge Risk: ⚪ Minimal · up to b7818

Unknown directory types now receive -32602 without changing the directory setting, while recognized values retain their existing mappings; no actionable merge risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b7818

The change prevents unrecognized directory types from changing a server’s directory setting. It changes the API response for those requests, but no new security exposure was identified. Runtime behavior has not been independently verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The affected security-relevant outcome is the authenticated server’s directory registration state: rejected type strings no longer reach its registration-capable setters. Recognized types retain that capability.

Security Findings and Attack Paths

• observed — The former path from an unrecognized type to the default directory type is closed by an early return. No introduced attack path was established in the inspected handler and dispatch path.

Trust Boundaries and Controls

• observed — Authentication is checked before method dispatch. Within the handler, string-type, recognized-value, and custom-address preconditions run before state-changing calls.

Resilience and Maintainability Implications

• observed — Accepted requests still use the pre-existing, non-transactional setter sequence. A string address can be stored before registration succeeds; that behavior is not introduced by this change.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Issue [#3915] requires setDirectory to reject unrecognised directoryType values with JSON-RPC error -32602 and leave the directory setting unchanged. The PR changes DeserializeDirectoryType to…
Out of Scope Changes check	✅ Passed	The reported changes are limited to the setDirectory parsing and rejection behavior, its public declaration, related JSON-RPC documentation, and the changelog. These changes support implementation, …
Title check	✅ Passed	The title clearly and concisely describes the main change: rejecting unrecognised directoryType values in jamulusserver/setDirectory.
Description check	✅ Passed	The description covers all required template sections, including the change summary, changelog entry, issue context, documentation impact, status, remaining work, testing details, and completed checkl…

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}