Skip to content

Commit 117fa73

Browse files
authored
Fix IETF parameter parsing. (#651)
1 parent 29415ab commit 117fa73

File tree

15 files changed

+156
-35
lines changed

15 files changed

+156
-35
lines changed

js/moq/src/ietf/parameters.ts

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { Reader, Writer } from "../stream";
2+
import { setVint62 } from "../stream";
23

34
export class Parameters {
45
entries: Map<bigint, Uint8Array>;
@@ -29,8 +30,17 @@ export class Parameters {
2930
await w.u53(this.entries.size);
3031
for (const [id, value] of this.entries) {
3132
await w.u62(id);
32-
await w.u53(value.length);
33-
await w.write(value);
33+
// Per draft-ietf-moq-transport-14 Section 1.4.2:
34+
// - If Type is even, Value is a single varint (no length prefix)
35+
// - If Type is odd, Value has a length prefix followed by bytes
36+
if (id % 2n === 0n) {
37+
// Even: value is stored as encoded varint bytes, write them directly
38+
await w.write(value);
39+
} else {
40+
// Odd: encode as length-prefixed bytes
41+
await w.u53(value.length);
42+
await w.write(value);
43+
}
3444
}
3545
}
3646

@@ -40,8 +50,23 @@ export class Parameters {
4050

4151
for (let i = 0; i < count; i++) {
4252
const id = await r.u62();
43-
const size = await r.u53();
44-
const value = await r.read(size);
53+
54+
// Per draft-ietf-moq-transport-14 Section 1.4.2:
55+
// - If Type is even, Value is a single varint (no length prefix)
56+
// - If Type is odd, Value has a length prefix followed by bytes
57+
let value: Uint8Array;
58+
if (id % 2n === 0n) {
59+
// Even: read varint and store as encoded bytes
60+
const varintValue = await r.u62();
61+
// Encode the varint back to bytes to store it
62+
const temp = new Uint8Array(8);
63+
const encoded = setVint62(temp.buffer, varintValue);
64+
value = encoded;
65+
} else {
66+
// Odd: read length-prefixed bytes
67+
const size = await r.u53();
68+
value = await r.read(size);
69+
}
4570

4671
if (params.entries.has(id)) {
4772
throw new Error(`duplicate parameter id: ${id.toString()}`);

rs/moq/src/coding/mod.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
33
mod decode;
44
mod encode;
5-
mod parameters;
65
mod reader;
76
mod size;
87
mod stream;
@@ -12,7 +11,6 @@ mod writer;
1211

1312
pub use decode::*;
1413
pub use encode::*;
15-
pub use parameters::*;
1614
pub use reader::*;
1715
pub use size::*;
1816
pub use stream::*;

rs/moq/src/ietf/fetch.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
use std::borrow::Cow;
22

33
use crate::{
4-
coding::{Decode, DecodeError, Encode, Parameters},
4+
coding::{Decode, DecodeError, Encode},
55
ietf::{
66
namespace::{decode_namespace, encode_namespace},
7-
GroupOrder, Location, Message,
7+
GroupOrder, Location, Message, Parameters,
88
},
99
Path,
1010
};

rs/moq/src/ietf/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ mod group;
55
mod location;
66
mod message;
77
mod namespace;
8+
mod parameters;
89
mod publish;
910
mod publish_namespace;
1011
mod publisher;
@@ -22,6 +23,7 @@ pub use goaway::*;
2223
pub use group::*;
2324
pub use location::*;
2425
pub use message::*;
26+
pub use parameters::*;
2527
pub use publish::*;
2628
pub use publish_namespace::*;
2729
use publisher::*;

rs/moq/src/ietf/parameters.rs

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
use std::collections::HashMap;
2+
3+
use crate::coding::*;
4+
5+
const MAX_PARAMS: u64 = 64;
6+
7+
#[derive(Default, Debug, Clone)]
8+
pub struct Parameters(HashMap<u64, Vec<u8>>);
9+
10+
impl Decode for Parameters {
11+
fn decode<R: bytes::Buf>(mut r: &mut R) -> Result<Self, DecodeError> {
12+
let mut map = HashMap::new();
13+
14+
// I hate this encoding so much; let me encode my role and get on with my life.
15+
let count = u64::decode(r)?;
16+
17+
if count > MAX_PARAMS {
18+
return Err(DecodeError::TooMany);
19+
}
20+
21+
for _ in 0..count {
22+
let kind = u64::decode(r)?;
23+
24+
if map.contains_key(&kind) {
25+
return Err(DecodeError::Duplicate);
26+
}
27+
28+
// Per draft-ietf-moq-transport-14 Section 1.4.2:
29+
// - If Type is even, Value is a single varint (no length prefix)
30+
// - If Type is odd, Value has a length prefix followed by bytes
31+
let data = if kind % 2 == 0 {
32+
// Even: decode as varint and encode it as bytes
33+
let value = u64::decode(&mut r)?;
34+
// Store the varint as bytes (we'll need to encode it back when accessing)
35+
let mut bytes = Vec::new();
36+
value.encode(&mut bytes);
37+
bytes
38+
} else {
39+
// Odd: decode as length-prefixed bytes
40+
Vec::<u8>::decode(&mut r)?
41+
};
42+
43+
map.insert(kind, data);
44+
}
45+
46+
Ok(Parameters(map))
47+
}
48+
}
49+
50+
impl Encode for Parameters {
51+
fn encode<W: bytes::BufMut>(&self, w: &mut W) {
52+
self.0.len().encode(w);
53+
54+
for (kind, value) in self.0.iter() {
55+
kind.encode(w);
56+
// Per draft-ietf-moq-transport-14 Section 1.4.2:
57+
// - If Type is even, Value is a single varint (no length prefix)
58+
// - If Type is odd, Value has a length prefix followed by bytes
59+
if kind % 2 == 0 {
60+
// Even: value is stored as encoded varint bytes, write them directly
61+
w.put_slice(value);
62+
} else {
63+
// Odd: encode as length-prefixed bytes
64+
value.encode(w);
65+
}
66+
}
67+
}
68+
}
69+
70+
impl Parameters {
71+
pub fn get(&self, kind: u64) -> Option<&Vec<u8>> {
72+
self.0.get(&kind)
73+
}
74+
75+
pub fn set(&mut self, kind: u64, value: Vec<u8>) {
76+
self.0.insert(kind, value);
77+
}
78+
}

rs/moq/src/ietf/publish.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,10 @@ The namespace or track is not of interest to the endpoint.
107107
use std::borrow::Cow;
108108

109109
use crate::{
110-
coding::{Decode, DecodeError, Encode, Parameters},
110+
coding::{Decode, DecodeError, Encode},
111111
ietf::{
112112
namespace::{decode_namespace, encode_namespace},
113-
GroupOrder, Location, Message,
113+
GroupOrder, Location, Message, Parameters,
114114
},
115115
Path,
116116
};

rs/moq/src/ietf/publish_namespace.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22
33
use std::borrow::Cow;
44

5-
use crate::{coding::*, ietf::Message, Path};
5+
use crate::{
6+
coding::*,
7+
ietf::{Message, Parameters},
8+
Path,
9+
};
610

711
use super::namespace::{decode_namespace, encode_namespace};
812

rs/moq/src/ietf/setup.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
use crate::{coding::*, ietf::Message};
1+
use crate::{
2+
coding::*,
3+
ietf::{Message, Parameters},
4+
};
25

36
/// Sent by the client to setup the session.
47
#[derive(Debug, Clone)]

rs/moq/src/ietf/subscribe.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use num_enum::{IntoPrimitive, TryFromPrimitive};
66

77
use crate::{
88
coding::*,
9-
ietf::{GroupOrder, Location, Message},
9+
ietf::{GroupOrder, Location, Message, Parameters},
1010
Path,
1111
};
1212

rs/moq/src/ietf/subscribe_namespace.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22
33
use std::borrow::Cow;
44

5-
use crate::{coding::*, ietf::Message, Path};
5+
use crate::{
6+
coding::*,
7+
ietf::{Message, Parameters},
8+
Path,
9+
};
610

711
use super::namespace::{decode_namespace, encode_namespace};
812

0 commit comments

Comments
 (0)