Closed Bug 1432934 Opened 3 years ago Closed 2 years ago

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

Categories

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

58 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: dminor, Assigned: johannes.willbold)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

[ 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)
Assignee: nobody → johannes.willbold
Comment on attachment 8983214 [details]
Bug 1432934: Added sanity check for recvonly attribute.

https://reviewboard.mozilla.org/r/249064/#review255536
Attachment #8983214 - Flags: review?(docfaraday) → review+
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 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.
Pushed by drno@ohlmeier.org:
https://hg.mozilla.org/integration/autoland/rev/a3a7ac1e38f6
Added sanity check for recvonly attribute. r=bwc
https://hg.mozilla.org/mozilla-central/rev/a3a7ac1e38f6
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.