facebookexperimental / facebookexperimental/moxygen
Interop client sends non-standard LargestGroup filter in draft-18 SUBSCRIBE
- Dominant language
- C++
- Stars
- 111
- Forks
- 35
- Avg merge
- 4m
- Merged PRs (30d)
- 1
Description
## Summary
`moq_interop_client` sends the non-standard `LocationType::LargestGroup` subscription filter value `250` in its draft-18 `SUBSCRIBE` messages. A draft-18 peer is required to reject that value with `PROTOCOL_VIOLATION`, so the `announce-subscribe` interop test fails against a conforming relay before namespace routing can be exercised.
This affects the moxygen client image directly. The moqx interop client also packages moxygen's `moq_interop_client`, so it shows the same failure.
## Reproduction
Run `englishm/moq-interop-runner` with either of these clients against `moq-dev-rs` from `origin/dev`:
- `ghcr.io/facebookexperimental/moxygen-interop-client:latest-amd64`
- `ghcr.io/openmoq/moqx-interop-client:latest`
Both negotiate `moqt-18`. `announce-only` completes successfully, then `announce-subscribe` fails immediately:
```text
not ok 4 - announce-subscribe
error: Subscribe failed: Session closed
```
The relay reports:
```text
moq_net::ietf::session: session error err=invalid value
```
## Root cause
The interop tests call:
```cpp
auto sub = SubscribeRequest::make(ftn);
```
The default argument is:
```cpp
LocationType locType = LocationType::LargestGroup
```
and `LargestGroup` is assigned value `250`. For draft 15 and newer, `writeSubscribeRequestHelper()` always serializes this as a `SUBSCRIPTION_FILTER` parameter.
Draft-18 defines only filter types `0x1` through `0x4` and requires an endpoint receiving another value to close the session with `PROTOCOL_VIOLATION`. The separate Largest Group extension currently assigns `0x20` and requires successful SETUP negotiation by both peers, so value `250` is not valid through that extension either.
## Suggested fix
Have the interop tests explicitly use a standard draft-18 filter such as `LocationType::LargestObject` or `LocationType::NextGroupStart`. Since these tests publish no objects before subscribing, either starts at `{0, 0}`.
It would also help to tighten `subscribe-error` and `subscribe-before-announce`: both currently treat `Session closed` as an acceptable request error, which makes them pass despite sending the same invalid `SUBSCRIBE`. They should require a protocol-level `REQUEST_ERROR` where that is the expected result.
(written by GPT-5)
Contributor guide
Assessment
This issue has not been assessed yet.