Skip to content

Commit fc6c45f

Browse files
committed
Fix draft-14 publish control lengths
1 parent fbcb78b commit fc6c45f

6 files changed

Lines changed: 15 additions & 23 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ Current status as of March 13, 2026:
199199
- with `--forward 0`, the current client then waits for inbound `SUBSCRIBE_NAMESPACE` / `SUBSCRIBE`
200200
- the Cloudflare endpoints accepted setup and namespace announce in testing, but did not issue subscriptions, so the publish attempt timed out waiting for control-stream data
201201
- with `--forward 1`, `moq-relay.red5.net:8443` now progresses through `PUBLISH_OK` for the catalog and media tracks, after which the client begins sending object streams
202-
- `fb.mvfst.net:9448` accepts QUIC for draft-14 and draft-16, but the current draft-14 flow stalls after namespace acceptance and the current draft-16 flow is still rejected with MOQT application error `3` (`PROTOCOL_VIOLATION`) immediately after setup
202+
- `fb.mvfst.net:9448` now accepts the draft-14 publish flow end-to-end after switching `PUBLISH`, `PUBLISH_OK`, and `PUBLISH_ERROR` control messages to `u16` outer lengths; the current draft-16 flow is still rejected with MOQT application error `3` (`PROTOCOL_VIOLATION`) immediately after setup
203203
- `--paced` only affects media-object sends; it does not delay setup, namespace announce, or track publish requests
204204

205205
### Optional picoquic smoke test

docs/protocol-mapping.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ This project keeps `draft-ietf-moq-transport-14` as the primary publisher profil
77
- Namespace subscription responses are modeled as the dedicated `SUBSCRIBE_NAMESPACE_OK` and `SUBSCRIBE_NAMESPACE_ERROR` flow.
88
- Namespace overlap handling is documented against `NAMESPACE_PREFIX_OVERLAP`.
99
- Publisher-side namespace acceptance is modeled with draft-14 style `PUBLISH_NAMESPACE_OK` and `PUBLISH_NAMESPACE_ERROR`.
10+
- Draft-14 control messages use a `u16` outer `Length` field, including `PUBLISH`, `PUBLISH_OK`, and `PUBLISH_ERROR`; only inner fields explicitly marked `(i)` remain QUIC varints.
1011

1112
## Draft-16 secondary assumptions
1213

include/openmoq/publisher/transport/moqt_control_messages.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,6 @@ std::vector<std::uint8_t> encode_object_stream(DraftVersion draft,
127127
bool decode_publish_namespace_ok(std::span<const std::uint8_t> bytes, PublishNamespaceOk& message);
128128
bool decode_publish_namespace_error(std::span<const std::uint8_t> bytes, PublishNamespaceError& message);
129129
bool decode_publish_ok(std::span<const std::uint8_t> bytes, DraftVersion draft, PublishOk& message);
130-
bool decode_publish_error(std::span<const std::uint8_t> bytes, PublishError& message);
130+
bool decode_publish_error(std::span<const std::uint8_t> bytes, DraftVersion draft, PublishError& message);
131131

132132
} // namespace openmoq::publisher::transport

src/transport/moqt_control_messages.cpp

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -257,8 +257,10 @@ bool next_control_message(std::span<const std::uint8_t> bytes, std::size_t& mess
257257
case kPublishType:
258258
case kPublishOkType:
259259
case kPublishErrorType: {
260-
if ((type == kPublishOkType || type == kPublishErrorType || type == kPublishType) &&
261-
offset + 2 <= bytes.size() && bytes[offset] == 0) {
260+
if (type == kPublishType || type == kPublishOkType || type == kPublishErrorType) {
261+
if (offset + 2 > bytes.size()) {
262+
return false;
263+
}
262264
const std::size_t payload_length =
263265
(static_cast<std::size_t>(bytes[offset]) << 8) | static_cast<std::size_t>(bytes[offset + 1]);
264266
message_size = offset + 2 + payload_length;
@@ -626,11 +628,7 @@ std::vector<std::uint8_t> encode_track_message(const TrackMessage& message) {
626628

627629
std::vector<std::uint8_t> message_bytes;
628630
append_varint(message_bytes, kPublishType);
629-
if (message.draft == DraftVersion::kDraft14) {
630-
append_varint(message_bytes, payload.size());
631-
} else {
632-
append_uint16(message_bytes, static_cast<std::uint16_t>(payload.size()));
633-
}
631+
append_uint16(message_bytes, static_cast<std::uint16_t>(payload.size()));
634632
message_bytes.insert(message_bytes.end(), payload.begin(), payload.end());
635633
return message_bytes;
636634
}
@@ -717,10 +715,7 @@ bool decode_publish_namespace_error(std::span<const std::uint8_t> bytes, Publish
717715
bool decode_publish_ok(std::span<const std::uint8_t> bytes, DraftVersion draft, PublishOk& message) {
718716
std::size_t payload_offset = 0;
719717
std::size_t payload_length = 0;
720-
const bool parsed =
721-
draft == DraftVersion::kDraft14 ? parse_varint_length_message(bytes, kPublishOkType, payload_offset, payload_length)
722-
: parse_uint16_length_message(bytes, kPublishOkType, payload_offset, payload_length);
723-
if (!parsed) {
718+
if (!parse_uint16_length_message(bytes, kPublishOkType, payload_offset, payload_length)) {
724719
return false;
725720
}
726721
if (payload_offset + payload_length > bytes.size()) {
@@ -791,10 +786,11 @@ bool decode_publish_ok(std::span<const std::uint8_t> bytes, DraftVersion draft,
791786
return offset == payload_end;
792787
}
793788

794-
bool decode_publish_error(std::span<const std::uint8_t> bytes, PublishError& message) {
789+
bool decode_publish_error(std::span<const std::uint8_t> bytes, DraftVersion draft, PublishError& message) {
790+
static_cast<void>(draft);
795791
std::size_t payload_offset = 0;
796792
std::size_t payload_length = 0;
797-
if (!parse_varint_length_message(bytes, kPublishErrorType, payload_offset, payload_length)) {
793+
if (!parse_uint16_length_message(bytes, kPublishErrorType, payload_offset, payload_length)) {
798794
return false;
799795
}
800796
if (payload_offset + payload_length > bytes.size()) {

src/transport/moqt_session.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ void trace_control_message(std::span<const std::uint8_t> message_bytes, openmoq:
137137
}
138138
} else if (message_type == 0x1f) {
139139
PublishError message;
140-
if (decode_publish_error(message_bytes, message)) {
140+
if (decode_publish_error(message_bytes, draft, message)) {
141141
std::cerr << " request_id=" << message.request_id << " error_code=" << message.error_code
142142
<< " reason=" << message.reason;
143143
}
@@ -262,7 +262,7 @@ TransportStatus collect_control_acknowledgements(PublisherTransport& transport,
262262
++publish_responses;
263263
} else if (message_type == 0x1f) {
264264
PublishError message;
265-
if (!decode_publish_error(message_bytes, message)) {
265+
if (!decode_publish_error(message_bytes, draft, message)) {
266266
return TransportStatus::failure("received invalid PUBLISH_ERROR");
267267
}
268268
return TransportStatus::failure("peer rejected track publish: " + message.reason);

tests/moqt_session_test.cpp

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -208,12 +208,7 @@ std::vector<std::uint8_t> encode_publish_ok_message(DraftVersion draft, std::uin
208208
}
209209

210210
std::vector<std::uint8_t> message = encode_varint(0x1e);
211-
if (draft == DraftVersion::kDraft14) {
212-
const std::vector<std::uint8_t> length = encode_varint(payload.size());
213-
message.insert(message.end(), length.begin(), length.end());
214-
} else {
215-
append_be16(message, static_cast<std::uint16_t>(payload.size()));
216-
}
211+
append_be16(message, static_cast<std::uint16_t>(payload.size()));
217212
message.insert(message.end(), payload.begin(), payload.end());
218213
return message;
219214
}

0 commit comments

Comments
 (0)