facebookexperimental / facebookexperimental/moxygen

getMoqtProtocols() preserves caller order: an ascending --versions list advertises moq-00 first and pins sessions to draft-14

Open
#219 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
111
Forks
35
Avg merge
4m
Merged PRs (30d)
1

Description

`getMoqtProtocols()` preserves the caller's numeric order, and `getAlpnFromVersion(14, useStandard=true)` returns the legacy ALPN because the `draftNum < 15` branch precedes the `useStandard` branch. A version list written in ascending order therefore advertises `moq-00` **first**, and every session silently pins to draft-14.

## Reproduction

`moqrelayserver` on localhost, identical client build in every row, client offering `moqt-18, moqt-16, moqt-15, moq-00`:

| relay `--versions` | resulting server ALPN list | negotiated |
|---|---|---|
| *(unset — default)* | `moqt-18, moqt-16, moqt-15, moq-00` | draft-18 |
| `18,16,14` | `moqt-18, moqt-16, moq-00` | draft-18 |
| `14,16,18` | `moq-00, moqt-16, moqt-18` | **draft-14** |

Selection follows server preference, so the relay's list order decides the outcome. The same effect is observable against `fb.mvfst.net:9448`, which negotiates draft-14 with a client offering all four ALPNs but draft-18 when the client pins `moqt-18`.

## Why this is easy to hit

Ascending is the natural way to write the flag, and it matches how draft support is conventionally listed elsewhere — the interop runner registry, for instance, lists `draft_versions: ["draft-14","draft-16","draft-18"]`. Nothing in the flag's help text (`"Comma-separated MoQ draft versions (e.g. '14,16'). Empty = all supported."`) suggests order is significant, and there is no diagnostic when the legacy ALPN ends up ahead of the standard ones.

## Not a request to change the published defaults

`getDefaultMoqtProtocols(true, ...)` already emits `moqt-18, moqt-16, moqt-15, moq-00`, which is correct, and the empty-`versions` path routes to it. The unset-flag behaviour is fine as shipped. This is only about the explicit-list path, where the caller's ordering silently becomes load-bearing.

## Options

Listed for your judgement rather than as a preference — the tradeoffs around fielded behaviour here are yours:

1. Sort the result by descending draft regardless of input order.
2. Keep input order, but log a warning when a legacy ALPN precedes a standard one.
3. Keep the behaviour and document in the flag help that order expresses preference, most-preferred first.

There is currently no test coverage for `MoQVersions` — no `getMoqtProtocols`, `getAlpnFromVersion`, `getVersionFromAlpn`, or `getDefaultMoqtProtocols` test anywhere in the tree. Happy to add a characterisation test for whichever behaviour you settle on.

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.