Skip to content
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

Inconsistency in ETag handling #155

Closed
jech opened this issue Dec 19, 2023 · 6 comments
Closed

Inconsistency in ETag handling #155

jech opened this issue Dec 19, 2023 · 6 comments

Comments

@jech
Copy link

jech commented Dec 19, 2023

Section 4.1.2:

A WHIP client sending a PATCH request for performing trickle ICE MUST
include an "If-Match" header field with the latest known entity-tag
as per [RFC9110] Section 13.1.1.

However, if the server does not support ICE restarts, then there might be no entity tag. Section 4.1.1:

it MUST generate a unique strong entity-tag identifying the
ICE session as per [RFC9110] Section 8.8.3, being OPTIONAL otherwise.

(By the way, the grammar of this last clause seems off to my untrained eyes.)

@murillo128
Copy link
Collaborator

murillo128 commented Dec 21, 2023

The last sentence was suggested by a directorate review, so I would be prone to keep it as it is.

How about the following text?

If the WHIP session supports ICE restarts, a WHIP client sending a PATCH request for performing trickle ICE MUST
include an "If-Match" header field with the latest known entity-tag
as per [RFC9110] Section 13.1.1.

@jech
Copy link
Author

jech commented Dec 21, 2023

If the WHIP session supports ICE restarts

I'm not sure. If the server supports ICE restarts and the client doesn't, then the session doesn't support ICE restarts, but sending If-Match is required.

The last sentence was suggested by a directorate review, so I would be prone to keep it as it is.

It's up to you, of course, but I believe it's grammatically incorrect.

@murillo128
Copy link
Collaborator

I'm not sure. If the server supports ICE restarts and the client doesn't, then the session doesn't support ICE restarts, but sending If-Match is required.

Yes, that's correct, the client always has to send the Etag

@jech
Copy link
Author

jech commented Dec 22, 2023

Could you please make that explicit in the text?

@murillo128
Copy link
Collaborator

If the WHIP session supports ICE restarts, a WHIP client sending a PATCH request for performing trickle ICE MUST
include an "If-Match" header field with the latest known entity-tag
as per [RFC9110] Section 13.1.1.

In fact, this is wrong, as the WHIP session may also send the entity-tag even if it doesn't support ICE restarts (it is OPTIONAL).

So it should be something like:

If the WHIP session is using entity-tags for identifying the ICE sessions in explained in Section 4.1.1, a WHIP client sending a PATCH request for performing trickle ICE MUST
include an "If-Match" header field with the latest known entity-tag
as per [RFC9110] Section 13.1.1.

@jech
Copy link
Author

jech commented Dec 26, 2023

Agreed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants