-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding an Extensible Object Header #502
base: main
Are you sure you want to change the base?
Changes from 6 commits
a5208af
0af94ca
8ccf41c
e9d83ce
7b150f7
8b5c167
358c5d7
ab407f7
3759564
e176497
6f31642
915ff95
ae71406
7488980
f2fba0c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -984,6 +984,14 @@ client MUST set the PATH parameter to the `path-abempty` portion of the | |
URI; if `query` is present, the client MUST concatenate `?`, followed by | ||
the `query` portion of the URI to the parameter. | ||
|
||
#### REQUIRED-EXTENSION parameter {#required-extensions} | ||
|
||
The REQUIRED-EXTENSION parameter (key 0x02) allows the client to specify | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a potentially painful way to negotiate extensions. What about making it REQUESTED-EXTENSION and then the server responds with the ones it supports of the requested set? At that point, if the client wants to close the connection because the server doesn't support a required extension, then it can. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ianswett - I have modified the PR with this commit to modify the extension negotiation and to rename the parameter to REQUESTED_EXTENSION
|
||
multiple Extension Header types {{object-extensions}} which are required for | ||
operation. The value is a concatenation of varints, each describing a | ||
32-bit extension header type. This parameter is optional. If the server does | ||
not support a requested REQUIRED-EXTENSION, then it MUST close the connection. | ||
wilaw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not too sure of the purpose of this parameter. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Imagine in the future we introduce a new header which signals to a relay that it should transmit this object in its own QUIC stream (for whatever reason). By "supporting" this header, the publisher is signalling that it is aware of any relay processing rules which that header may invoke. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about we have something along the lines in the core transport ( below formulation ,may be, exactly what you have, but i am writing it down in a single place to see if we all can agree) Relay Extension Processing Rules
Now on the point on how to know if a relay understands an extension or not , there are couple of options
Extensions are per object property. I am not totally sure if we need stream level extensions at the moq layer though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Suhas - I agree on point 1 (processing rules are defined by whatever spec defines the extension).
A protocol violation can only be triggered by behaviors defined by the MOQT spec. It cannot be a protocol violation to not support some future optional extension. It may be a protocol violation of the 3rd party spec that defines the extension, but that spec will need to deal with that outcome as these are, after all, optional extensions as far as MOQT is concerned.
We have the setup negotiation already in place. This informs the client whether the server "supports" a given extension. By 'support' we mean that it understands any processing rules that are defined by the 3rd party spec. This can give the client some confidence that the server is likely to act in compliant manner regarding that extension. |
||
## GOAWAY {#message-goaway} | ||
The server sends a `GOAWAY` message to initiate session migration | ||
({{session-migration}}) with an optional URI. | ||
|
@@ -1511,6 +1519,12 @@ MUST be sent according to its `Object Forwarding Preference`, described below. | |
* Object Status: As enumeration used to indicate missing | ||
objects or mark the end of a group or track. See {{object-status}} below. | ||
|
||
* Object Extension Count: The number of Object Extensions present. A value of 0 | ||
indicates that no Object Extension Headers are present. | ||
|
||
* Object Extensions : an optional concatenation of Object Extension Headers. See | ||
{{object-extensions}} below. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to talk about ideas to bit pack the extension indictors to save space but conceptually good with this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like to understand what do we mean by concatenation here ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Meaning a series of Object Extension headers, serialized in sequence without any delimiters between them. |
||
|
||
* Object Payload: An opaque payload intended for an End Subscriber and SHOULD | ||
NOT be processed by a relay. Only present when 'Object Status' is Normal (0x0). | ||
|
||
|
@@ -1557,6 +1571,62 @@ are sent on a new stream. This is to avoid the status message being lost | |
in cases such as a relay dropping a group and reseting the stream the | ||
group is being sent on. | ||
|
||
#### Object Extension {#object-extensions} | ||
An Object Extension is a concatenation of optional Extension Headers. These | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am bit worried by the usage of word Optional here? What exactly is optional? Looking below I get the impression that Extension header handling are depending on role of the node but in no case are they truly optional, other than it comes to the publisher including them. A Relay must forward and not modify |
||
headers are visible to relays. Extension headers MUST be forwarded and | ||
MUST NOT be modified by relays. The purpose of Extension Headers is to | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I agree if the relay doesn't understand the extension then we need these normative requirements. I wonder if there's room for a negotiated extension that is solely between the publisher and the first hop, where the first hop is allowed to remove the extension if it not longer has applicability. |
||
allow the transmission of application-specific data as well as future | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
MoQT already allows for the transmission of application specific data -- object payloads. What's unique here is a) there can be some well known structure of the application data and b) it is visible to relays as well as the end consumer. |
||
evolution of the transport protocol. Object Extensions are serialized as | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Maybe reword to "modification of transport protocol behavior" |
||
defined below: | ||
|
||
~~~ | ||
Object Extension { | ||
Extension Header (..) ... | ||
} | ||
|
||
Extension Header { | ||
wilaw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Header Type (i), | ||
Header Length (i), | ||
Header Value (..) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Header Value should be opaque bytes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clarification - opaque to whom - the relays? Object headers values are not opaque to relays and nor are Extension Header values. If an application wants to hide content from a relay then it needs to place it in the Object payload (which is opaque) and not the headers. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a relay supports an extension and needs it for some thing, it can parse it. If not, it forwards it as-is. This will enable experimentation and also support real-usecase where relay needs to use the information. My inclination is each extension document defines its scope and usage. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think Suhas and Will are saying the same thing here. There's language above that says you MUST forward unmodified. |
||
} | ||
~~~ | ||
{: #object-extension-format title="Object Extension Header Format"} | ||
|
||
* Header type: an unsigned 32-bit integer, registered in the IANA table | ||
'MOQ Extension Headers'. See {{iana}}. | ||
wilaw marked this conversation as resolved.
Show resolved
Hide resolved
wilaw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
##### Extension Header type 0 {#extension-header-zero} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need this defined ? Can you please share your thoughts on why we need this ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This default extension header provides a simple way to add ad-hoc name/value pairs to any object. This is similar to the ability of clients and servers to define custom request and response headers in HTTP-land. That functionality has proven extremely useful and efficient in enabling rapid development, testing and deployment of solutions without having to go to IANA registries or revise specifications. I'd like a similar capability within MOQT hence Extension Header Type Zero. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the context. Still i don't see if this adds much value in end to end context. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on your and others feedback, I have removed (via commit e176497) the Extension header type 0 definition. It can be added back in the future if necessary. |
||
|
||
This specification defines a utility extension header. The value of this header | ||
wilaw marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this is kept in, it purpose needs to be clarified. But it feels like this is creating an additional extension space. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on your and others comments, I removed (via commit e176497) the Extension header type 0 definition. It can be added back in the future if necessary. |
||
is itself a name-value pair. | ||
|
||
|
||
| Type | Value | | ||
| ---- | ---------------------------------------------------- | | ||
| 0x0 | Header Value - see {{extension-header-zero-format}} | | ||
|
||
|
||
~~~ | ||
Header Value { | ||
Name Length (i), | ||
Name Value (..) | ||
Payload (..) | ||
} | ||
~~~ | ||
{: #extension-header-zero-format title="Extension header 0 value format"} | ||
|
||
* Name Length: the size of the name value in bytes. | ||
* Name Value: a string encoded using ISO-8859-1. This name is application-defined | ||
and is not IANA registered. | ||
* Payload: the contents of the header. The combined size of the name and payload | ||
contents MUST NOT exceed 10240 bytes. | ||
|
||
### Object Message Formats | ||
|
||
Every Track has a single 'Object Forwarding Preference' and the Original | ||
Publisher MUST NOT mix different forwarding preferences within a single track. | ||
If a subscriber receives different forwarding preferences for a track, it | ||
SHOULD close the session with an error of 'Protocol Violation'. | ||
|
||
## Object Stream | ||
|
||
|
@@ -1577,6 +1647,8 @@ OBJECT_STREAM Message { | |
Group ID (i), | ||
Object ID (i), | ||
Publisher Priority (8), | ||
Extension Count (i), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This adds 1 byte to every object including those with no extensions. I'd rather we handle this as a flag embedded in the stream header type (eg: STREAM_HEADER_SUBGROUP=0x02-03. If the LSB is 1 then Extension Count and one or more extensions are present). |
||
[Extension headers (...)], | ||
wilaw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Object Payload Length (i), | ||
[Object Status (i)], | ||
Object Payload (..), | ||
|
@@ -1613,6 +1685,8 @@ OBJECT_DATAGRAM Message { | |
Group ID (i), | ||
Object ID (i), | ||
Publisher Priority (8), | ||
Extension Count (i), | ||
[Extension headers (...)], | ||
Object Payload Length (i), | ||
[Object Status (i)], | ||
Object Payload (..), | ||
|
@@ -1771,6 +1845,38 @@ OBJECT_STREAM { | |
} | ||
~~~ | ||
|
||
Sending a group on one stream, with the first object containing an | ||
Extension Header. | ||
|
||
~~~ | ||
Stream = 2 | ||
|
||
STREAM_HEADER_GROUP { | ||
Subscribe ID = 2 | ||
Track Alias = 2 | ||
Group ID = 0 | ||
Publisher Priority = 0 | ||
} | ||
{ | ||
Object ID = 0 | ||
Extension Count = 1 | ||
{ Type = 0 | ||
Length = 21 | ||
{ Name length = 15 | ||
Name value = "example-traceID" | ||
Payload = "123456" | ||
} | ||
} | ||
Object Payload Length = 4 | ||
Payload = "abcd" | ||
} | ||
{ | ||
Object ID = 1 | ||
Object Payload Length = 4 | ||
Payload = "efgh" | ||
} | ||
|
||
~~~ | ||
|
||
|
||
# Security Considerations {#security} | ||
|
@@ -1806,6 +1912,7 @@ TODO: fill out currently missing registries: | |
* Subscribe Error codes | ||
* Announce Error codes | ||
* Message types | ||
* MOQ Extension headers | ||
|
||
TODO: register the URI scheme and the ALPN | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it makes sense to negotiate object extensions with the relay. This would be like negotiating HTTP header extensions with a socks proxy. Other types of extensions we need too negotiate with server but not these. I think we can just remove this part of the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This negotiation has now changed slightly. The clients uses the revised REQUESTED-EXTENSION parameter to indicate which extensions it would like and the server responds with the sub-set of those that it does support. The client may then chose to continue or disconnect.
Since we have separate rules that relays MUST always forward all headers and MUST NOT modify them, any negotiation is practically only going to happen at the final hop. I agree that its utility is limited. I would like to remove it, but first I want agreement from those who wanted it in there in the first place.