jamulussoftware / jamulussoftware/jamulus
jamulusserver/setDirectory silently substitutes any_genre_1 for an unrecognised directoryType and replies ok
- 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
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