jamulussoftware / jamulussoftware/jamulus

jamulusserver/setDirectory silently substitutes any_genre_1 for an unrecognised directoryType and replies ok

Offen
#3,915 1 Kommentar 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
C
Sterne
1.1k
Forks
248
Ø Merge
2 T. 3 Std.
Gemergte PRs (30 T.)
9

Beschreibung

**🤖 AI:** `jamulusserver/setDirectory` accepts any string as `directoryType` and replies `"ok"`. Unrecognised values are silently mapped to `any_genre_1`, the public default.

`DeserializeDirectoryType()` returns `AT_DEFAULT` when the lookup misses, and `sumStringToDirectoryType` is an `unordered_map` keyed on `std::string`, so matching is byte-exact: case-sensitive and not trimmed.

Measured on loopback with `*.jamulus.app` redirected to 127.0.0.1 in `/etc/hosts`, so no probe reached a real directory. Each row: set, then read back with `getServerProfile`, one request per fresh connection.

| `directoryType` sent | reply | resulting type | registered with |
|---|---|---|---|
| `custom` + address | ok | `custom` | `my.private.example:22124` |
| `CUSTOM` + address | ok | `any_genre_1` | `anygenre1.jamulus.app:22124` |
| `custon` + address | ok | `any_genre_1` | `anygenre1.jamulus.app:22124` |
| `custom ` (trailing space) | ok | `any_genre_1` | `anygenre1.jamulus.app:22124` |
| `custom`, no address | -32602 | unchanged | — |
| `CUSTOM`, no address | ok | `any_genre_1` | `anygenre1.jamulus.app:22124` |
| `GENRE_ROCK` | ok | `any_genre_1` | `anygenre1.jamulus.app:22124` |
| `genre_rockk` | ok | `any_genre_1` | `anygenre1.jamulus.app:22124` |
| `genre-rock` | ok | `any_genre_1` | `anygenre1.jamulus.app:22124` |
| `NONE` | ok | `any_genre_1` | `anygenre1.jamulus.app:22124` |
| `unknown_xyz` | ok | `any_genre_1` | `anygenre1.jamulus.app:22124` |
| `""` | ok | `any_genre_1` | `anygenre1.jamulus.app:22124` |

Two consequences.

**A typo moves a running server off its chosen genre.** Set `genre_jazz`, then send `GENRE_JAZZ`. The reply is `"ok"` and the server is now registered with `any_genre_1`.

**The `custom` guard cannot fire for the inputs it exists to catch.** `setDirectory` already rejects `custom` supplied without a `directoryAddress`, but that check tests the deserialized enum rather than the input string — so `"CUSTOM"` becomes `AT_DEFAULT` before the check runs. An operator asking for their own directory is registered with the public one, their `directoryAddress` stored and ignored, and the reply is `"ok"`.

The matrix above was measured against `main` @ `97799184`. I re-checked today: `DeserializeDirectoryType()` is byte-identical in `main` and in `r3_12_4`, so this is present unchanged in `release/3_12`.

I hit this in practice rather than only in testing — unregistering a server via `{"directoryType":"none"}` is one keystroke away from publishing it to the busiest directory on the network, with a success reply either way.

I have a patch that returns `-32602` for an unrecognised value, echoing it and listing the accepted set, and leaves the directory setting unchanged. It follows `jamulusclient/setSkillLevel`, which already rejects unrecognised enum strings this way. A/B against baseline across 24 cases of the server RPC surface: 22 identical, 2 intended differences, 0 unintended.

Before opening anything, one decision that is yours rather than mine.

Rejecting is a behaviour change on an API released in 3.12.0. A script sending a bad value today gets `"ok"`; afterwards it gets an error. My reading is that this is the point, since the current reply reports a success that did not happen — but it is a break, so:

- reject on `main` only and leave `release/3_12` as is,
- reject on both,
- or an alternative you prefer, such as keeping the fallback and reporting the substitution in the result.

Happy to open a PR once you've said which.

---

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

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Beginnen Sie am Einstiegspunkt jamulusserver/setDirectory und untersuchen Sie DeserializeDirectoryType() und sumStringToDirectoryType; vergleichen Sie deren Validierungsverhalten mit jamulusclient/setSkillLevel. Verwenden Sie den gemeldeten 24-Fälle-Vergleich der Server-RPCs als Testgrundlage und betrachten Sie die Entscheidung des Maintainers für main gegenüber release/3_12 als umgesetzt, wenn ungültige Werte einen Fehler zurückgeben, ohne die Verzeichniseinstellung zu ändern.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
cpp
Bereich
api, backend
Issue-Typ
Bug
Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Aktivitätsstatus
Aktiv
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
35/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.