sdp_ops: implement token-indexed writes for $sdp.line/.stream/.session - #4238
Open
Lt-Flash wants to merge 1 commit into
Open
sdp_ops: implement token-indexed writes for $sdp.line/.stream/.session#4238Lt-Flash wants to merge 1 commit into
Lt-Flash wants to merge 1 commit into
Conversation
The "handle_token_edit:" label in pv_set_sdp_line(), pv_set_sdp_stream()
and pv_set_sdp_session() was an empty stub: a token-indexed assignment
such as
$sdp.line(o=/[5]) = "0.0.0.0";
located the line, jumped to the label, did nothing, fell into
out_success, set SDP_OPS_FL_DIRTY and returned success. The edit never
materialised (the reporter's own $sdp print still shows the original IP)
while the DIRTY flag alone was enough to re-derive msg->body from
sdp_ops' buffer - which is what armed the crash fixed in OpenSIPS#4237. Token
indexing had been implemented in the getters and never in the setters.
Implemented now, with the semantics described in
docs/manual/Script-CoreVar.md: one sdp_ops_find_token() shared by the
readers and the writers, so "token N" cannot mean two different things,
and one sdp_ops_set_token() behind all three labels. A token prefix is a
key that survives the write ($sdp.line(a=fmtp:96/bitrate=) = "64000"
yields "bitrate=64000"); matched by bare index the whole token is the
value; deleting a token takes the whitespace on one side with it, and a
line left empty is dropped rather than kept blank. An embedded EOL in
the value is refused (a token lives inside one line), and [insert] /
[insertAfter] are refused on tokens, being line operations. The three
getters now use the shared finder instead of three copies of the walk.
Related to OpenSIPS#4135
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related to #4135 (companion to #4237, which fixes the crash itself).
$sdp.line(o=/[5]) = "0.0.0.0";— a token-indexed write — was a silent no-op: thehandle_token_edit:label inpv_set_sdp_line(),pv_set_sdp_stream()andpv_set_sdp_session()was an empty stub. The statement located the line, did nothing, setSDP_OPS_FL_DIRTYand returned success. Token indexing existed in the getters but was never implemented in the setters — which is also the answer to the second question in #4135 (o=/[4] = "IP6""flags dirty without visible changes"). The DIRTY flag alone was enough to re-derivemsg->bodyfrom sdp_ops' buffer, i.e. the no-op write is what armed the crash fixed in #4237.Implemented, following
docs/manual/Script-CoreVar.md:sdp_ops_find_token()shared by readers and writers, so "token N" cannot mean two different things; the three getters now use it instead of three copies of the token walk;sdp_ops_set_token()behind all threehandle_token_edit:labels;$sdp.line(a=fmtp:96/bitrate=) = "64000"yieldsbitrate=64000; matched by bare index (o=/[5]) the whole token is the value;[insert]/[insertAfter]are refused on tokens, being line operations.Verified with a 15-assertion script rig (read-side forms from the manual, replace by index, replace by prefix keeping the key, delete without double spaces, untouched lines intact, the
$sdp.sessionpath) — run first against the unpatched build, where every edit assertion failed withAFTER == BEFORE. Builds clean with-Wredundant-decls -Wold-style-definition -Wmissing-field-initializers -Werroron current master, independently of #4237.Two examples in the manual are self-inconsistent and were left alone:
m=audio[1]/[0]is documented asaudiobut the reader has always returnedm=audio, anda=nortpproxy:/[0] = "yes"cannot work under whitespace tokenisation.