facebookexperimental / facebookexperimental/moxygen

Interop client sends non-standard LargestGroup filter in draft-18 SUBSCRIBE

Open
#225 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.