OpenSIPS / opensips

OpenSIPS is a GPL implementation of a multi-functionality SIP Server that targets to deliver a high-level technical solution (performance, security and quality) to be used in professional SIP server platforms.
https://opensips.org
Other
1.28k stars 581 forks source link

[BUG] rtpengine module parse_flags error #2692

Closed wangduanduan closed 2 years ago

wangduanduan commented 2 years ago

OpenSIPS version you are running

version: opensips 3.2.3 (x86_64/linux)
flags: STATS: On, DISABLE_NAGLE, USE_MCAST, SHM_MMAP, PKG_MALLOC, Q_MALLOC, F_MALLOC, HP_MALLOC, DBG_MALLOC, FAST_LOCK-ADAPTIVE_WAIT
ADAPTIVE_WAIT_LOOPS=1024, MAX_RECV_BUFFER_SIZE 262144, MAX_LISTEN 16, MAX_URI_SIZE 1024, BUF_SIZE 65535
poll method support: poll, epoll, sigio_rt, select.
git revision: 671cc256a
main.c compiled on 09:01:19 Nov  8 2021 with gcc 8

Describe the bug opensips rtpengine bencode encode error, below is a packet from opensips to rtpeinge

  1. the root payload is a dict, but the last char is not "e"

request-to-rtpengine.pcap.zip

wangduanduan commented 2 years ago

copy form wireshark

143_2189_9 d3:sdp275:v=0
o=888888 8000 8000 IN IP4 192.168.1.106
s=SIP Call
c=IN IP4 192.168.1.106
t=0 0
m=audio 56290 RTP/AVP 0 8 101
a=sendrecv
a=rtcp:56291 IN IP4 192.168.1.106
a=rtpmap:0 PCMU/8000
a=ptime:20
a=rtpmap:8 PCMA/8000
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-15
3:ICE6:remove7:replacel18:session-connection6:origine18:transport-protocol7:RTP/AVP7:call-id33:1830075472-27660-10@BJC.BGI.B.BAG13:received-froml3:IP414:172.16.200.253e8:from-tag10:15484599617:command5:offer
wangduanduan commented 2 years ago

i use rtpengine_offer("media-address=...."), but from the packet , opensips not use the "media-address" as param to send to rtpengine

razvancrainea commented 2 years ago

There is indeed a bug regarding the media-address option. I will push a fix soon. But for the other reports, I honestly do not understand what is the problem. Can you please elaborate?

wangduanduan commented 2 years ago

not only the media-addres, may be all break and continue is wrong. all continue and break should switch their position.

please check this.

                      case 3:
                if (str_eq(&key, "RTP")) {
                    ng_flags->transport |= 0x100;
                    ng_flags->transport &= ~0x001;
                }
                else if (str_eq(&key, "AVP")) {
                    ng_flags->transport |= 0x100;
                    ng_flags->transport &= ~0x002;
                } else
                    continue;
                break;

            case 4:
                if (str_eq(&key, "SRTP"))
                    ng_flags->transport |= 0x101;
                else if (str_eq(&key, "AVPF"))
                    ng_flags->transport |= 0x102;
                else
                    continue;
                break;
                      ...

if continue is last, the blow is never be exected, it will continue next loop.

               /* we got here if we didn't match something specific */
        if (!val.s) {
            bitem = bencode_str(bencode_item_buffer(ng_flags->flags), &key);
            if (!bitem) {
                err = "no more memory";
                goto error;
            }
            BCHECK(bencode_list_add(ng_flags->flags, bitem));
        } else {
            bitem = bencode_str(bencode_item_buffer(ng_flags->dict), &val);
            if (!bitem) {
                err = "no more memory";
                goto error;
            }
            BCHECK(bencode_dictionary_add_len(ng_flags->dict, key.s, key.len, bitem));
        }
github-actions[bot] commented 2 years ago

Any updates here? No progress has been made in the last 15 days, marking as stale. Will close this issue if no further updates are made in the next 30 days.

github-actions[bot] commented 2 years ago

Marking as closed due to lack of progress for more than 30 days. If this issue is still relevant, please re-open it with additional details.

razvancrainea commented 2 years ago

IMO the continue should be the last - break + the flags block should only be executed if the transport does not match. So I think the code is ok as it is now. Can you please confirm?

github-actions[bot] commented 2 years ago

Any updates here? No progress has been made in the last 15 days, marking as stale. Will close this issue if no further updates are made in the next 30 days.

github-actions[bot] commented 2 years ago

Marking as closed due to lack of progress for more than 30 days. If this issue is still relevant, please re-open it with additional details.