1432934 - Rust SDP Parser fails to produce an error on NewSdpTest.ParseInvalidSimulcastNotSending

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect, P3)

Description

[Dan Minor [:dminor]]

[ RUN      ] RoundTripSerialize/NewSdpTest.ParseInvalidSimulcastNotSending/0
version: 0
origin: -, 4294967296, 2, 127.0.0.1
session: SIP Call
connection: 198.51.100.7
bandwidth: CT, 5000
timing: 0, 0
media: Video, 56436, Rtp/Savpf, [120]
/home/dminor/src/firefox/media/webrtc/signaling/gtest/sdp_unittests.cpp:3783: Failure
Expected: ("") != (GetParseErrors()), actual: "" vs ""
[  FAILED  ] RoundTripSerialize/NewSdpTest.ParseInvalidSimulcastNotSending/0, where GetParam() = (false, false) (1 ms)

Comment 1

[Johannes Willbold]

Attachment: Bug 1432934: Added sanity check for recvonly attribute.

Added a sanity check, that checks for recvonly and whether simulcast defines send options

Review commit: https://reviewboard.mozilla.org/r/249064/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/249064/

Comment 2

[Byron Campen [:bwc]]

Comment on attachment 8983214 [details]
Bug 1432934: Added sanity check for recvonly attribute.

https://reviewboard.mozilla.org/r/249064/#review255536

Comment 3

[Nils Ohlmeier [:drno]]

Comment on attachment 8983214 [details]
Bug 1432934: Added sanity check for recvonly attribute.

https://reviewboard.mozilla.org/r/249064/#review256646

I think this needs a little fix in the error message before we land it.

::: media/webrtc/signaling/src/sdp/rsdparsa/src/lib.rs:623
(Diff revision 1)
>          }
> +        if msection.get_attribute(SdpAttributeType::Recvonly).is_some() {
> +            if let Some(&SdpAttribute::Simulcast(ref x)) = msection.get_attribute(SdpAttributeType::Simulcast) {
> +                if x.send.len() > 0 {
> +                    return Err(SdpParserError::Sequence {
> +                        message: "Simulcast can't define send parameters for recvonly".to_string(),

Why does this have the same error message as the check above?

Comment 4

[Johannes Willbold]

Comment on attachment 8983214 [details]
Bug 1432934: Added sanity check for recvonly attribute.

https://reviewboard.mozilla.org/r/249064/#review256646

> Why does this have the same error message as the check above?

This error message is correct, the above is incorrect and has already been fixed in another commit.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a3a7ac1e38f6
Added sanity check for recvonly attribute. r=bwc

Comment 6

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/a3a7ac1e38f6