Ignore malformed candidates

[philipch07]

Description

Ignore malformed candidates instead of returning an error.

Reference issue

Resolves pion/webrtc#1315. This is related to #216, but does not resolve it.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.57%. Comparing base (b8e35f4) to head (bfe121f).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #233      +/-   ##
==========================================
+ Coverage   97.54%   97.57%   +0.03%     
==========================================
  Files          12       12              
  Lines        1385     1403      +18     
==========================================
+ Hits         1351     1369      +18     
  Misses         18       18              
  Partials       16       16              

Flag	Coverage Δ
go	97.57% <100.00%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[JoeTurki]

Looks good to me, the less strict we're the better.
but we have to make sure that we don't panic somewhere else.
Did you test that? I can help with testing.

[philipch07]

Looks good to me, the less strict we're the better. but we have to make sure that we don't panic somewhere else. Did you test that? I can help with testing.

No, I haven't gotten around to testing this in-depth yet. I added some fuzz tests though to hopefully catch if anything goes wrong, but you do bring up an important point to make sure we don't panic. If you want to help add more tests for that it would be very welcome!!

[JoeTurki]

@philipch07 panic in SDP isn't the concern here, how webrtc and ICE down the stack handles these malformed candidates, Do you know?

[philipch07]

@philipch07 panic in SDP isn't the concern here, how webrtc and ICE down the stack handles these malformed candidates, Do you know?

My current understanding from reading the discussion in the linked issues is that the SDP originally threw errors on malformed/unrecognized candidates which caused subsequent candidates to not be parsed, is that correct? I'm not sure if that answers your question though.

[JoeTurki]

My current understanding from reading the discussion in the linked issues is that the SDP originally threw errors on malformed/unrecognized candidates which caused subsequent candidates to not be parsed, is that correct? I'm not sure if that answers your question though.

This is true, now if we make the parser less strict, we should also check that we don't have exceptions in other parts of the stack that can case it to panic. It can be something simple as: reading from an empty slice.

[philipch07]

It can be something simple as: reading from an empty slice.

Oh yeah that's a perfect example; we should definitely check for that. I won't be able to work on this until later tomorrow if you'd like to work on this. No worries if you're busy of course, and no pressure as always.

[JoeTurki]

@philipch07 I'll try to check ICE after i finish something, thank you.

[philipch07]

@philipch07 I'll try to check ICE after i finish something, thank you.

Awesome! I'm looking forward to reading up on what you come up with tomorrow!! Thanks so much