jamulussoftware / jamulussoftware/jamulus
jamulusserver/setDirectory silently substitutes any_genre_1 for an unrecognised directoryType and replies ok
- Lingua principale
- C
- Stelle
- 1.1k
- Fork
- 248
- Merge medio
- 2g 3h
- PR unite (30g)
- 9
Descrizione
**🤖 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.*
Guida per i contributori
Apri la guida per i contributori
Direzione di ricerca
Parti dal punto di ingresso jamulusserver/setDirectory ed esamina DeserializeDirectoryType() e sumStringToDirectoryType; confronta il loro comportamento di validazione con jamulusclient/setSkillLevel. Usa come base per i test il confronto riportato dei server RPC su 24 casi e considera completata la decisione del maintainer su main rispetto a release/3_12 quando i valori non validi restituiscono un errore senza modificare l'impostazione della directory.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- cpp
- Ambito
- api, backend
- Tipo di issue
- Bug
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Stato di attività
- Attiva
- Chiarezza
- Abbastanza chiara
- Idoneità per principianti
- 35/100