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

Fix: RTCSession.js UPDATE method flow #828

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

OliverGarfield
Copy link

@OliverGarfield OliverGarfield commented Jul 17, 2023

UPDATE is usually completed before the INVITE request is answered, so the 1XX status is added to the judgment processing UPDATE

The update is usually done before the invite request is answered, so the processing of 1XX states is added
@OliverGarfield OliverGarfield changed the title Update UPDATE method Fix: RTCSession.js UPDATE method flow Jul 17, 2023
@OliverGarfield
Copy link
Author

OliverGarfield commented Jul 17, 2023

RFC3311 defines the UPDATE method, which can modify session parameters without changing the session state. In SIP, an INVITE request is used to establish a session, and the offer/answer model is used in the process. Usually, an offer is included in the INVITE, and an answer is included in the response it replies. After the session is established, however, session parameters can be modified with re-INVITE. , before the session is pending (the INVITE has been issued and the final response has not been received), neither party can send a re-INVITE. If the session parameters need to be changed in the near future, the only way is to use the UPDATE method.
image

@OliverGarfield
Copy link
Author

@jmillan

@OliverGarfield
Copy link
Author

@jmillan Hi?

@ibc
Copy link
Member

ibc commented Aug 19, 2023

It's summer here and hence vacation days. We'll come to this and many other pending work on next weeks.

@OliverGarfield
Copy link
Author

It's summer here and hence vacation days. We'll come to this and many other pending work on next weeks.

Ok,I'm sorry to disturb your vacation.

@stefang42
Copy link
Contributor

stefang42 commented Sep 26, 2023

Hi @OliverGarfield! Sorry, but I think your modification is not quite correct. The UAS ("Beale" in your diagram) is not in state STATUS_1XX_RECEIVED when it receives the UPDATE request, because it's the other way around: the UAC ("Vigenere") that sent the INVITE received the provisional response. After receiving the INVITE and sending the provisional responses the UAS will be in state STATUS_WAITING_FOR_ANSWER, not STATUS_1XX_RECEIVED, when it receives the UPDATE.

(Also, looking at https://www.rfc-editor.org/rfc/rfc3311.html#section-5.2, I'm not sure JsSIP is already handling UPDATE requests correctly in every case, depending on the offers/answers already exchanged or not...
EDIT: Ok, looks like Dialog._checkInDialogRequest already takes care of rejecting the UPDATE in such cases.)

@OliverGarfield
Copy link
Author

@stefang42 You're welcome! Thank you for your response and for providing important clarifications regarding SIP (Session Initiation Protocol) and RFC 3311. I have read your points and understand your explanation.

You are absolutely correct in pointing out that the UAS ("Beale" in your diagram) typically enters the "STATUS_WAITING_FOR_ANSWER" state after receiving an INVITE request and sending provisional responses. In RFC 3311, it is indeed stated that the "UPDATE" request is replied to after entering the "STATUS_WAITING_FOR_ANSWER" state, contingent on the presence of a "PRACK" message. However, it is a valid consideration whether one should reply to the "UPDATE" in the "STATUS_1XX_RECEIVED" state if no "PRACK" message is present.

This is indeed a valid point, and the decision to respond to an "UPDATE" in the "STATUS_1XX_RECEIVED" state in the absence of a "PRACK" message may depend on specific implementations and requirements. It's important to consider the particular use case and how different implementations handle this scenario.

@stefang42
Copy link
Contributor

Hi @OliverGarfield! Sorry, maybe I did not express myself clearly before. I'm not necessarily saying your current patch is wrong. While I have never seen this scenario personally, there may indeed be cases where the caller that sent the INVITE may receive an UPDATE request after receiving a provisional response (i.e. the caller's JsSIP RTCSession is in state STATUS_1XX_RECEIVED), depending on whether offers/answers have been exchanged (though PRACK is currently not supported by JsSIP I think).

I'm just saying that in my opinion the current patch is not sufficient, because it doesn't cover the situation where the callee receives an UPDATE before generating the final response to the INVITE it received, i.e. the situation outlined in the diagram you attached. There on the callee side JsSIP's RTCSession will be in state STATUS_WAITING_FOR_ANSWER when it receives the UPDATE, which is a valid scenario, e.g. when no offer has been exchanged yet. So I think your patch should also cover that state, not only STATUS_1XX_RECEIVED.

@jmillan
Copy link
Member

jmillan commented Nov 2, 2023

Agreed with @stefang42, this PR needs more coverage for the given scenarios, at least.

@evilArsh
Copy link

It's summer here and hence vacation days. We'll come to this and many other pending work on next weeks.

i envy you have a good summer vacation

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

Successfully merging this pull request may close these issues.

5 participants