Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parser: Allow connection line to be specified multiple times #18

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nemosupremo
Copy link
Contributor

I've misplaced the wireshark output, but we ran into a situation where a camera's rtsp stream we were working with had specified the connection line multiple times; which caused sdp-types to fail parsing. This new just skips the duplicate lines instead. I can understand the rationale of failing on bad input, but I think the robustness principle applies.

@nemosupremo nemosupremo force-pushed the fix/allow-multiple-connection branch from b88412e to 3fd6af1 Compare December 31, 2023 19:00
@nemosupremo nemosupremo force-pushed the fix/allow-multiple-connection branch from 3fd6af1 to d9add17 Compare December 31, 2023 19:02
@sdroege
Copy link
Owner

sdroege commented Jan 1, 2024

That makes sense, and I think we should do the same for all the other cases like this (no need to do this as part of this PR).

It would probably also make sense to add a dependency on the log crate and add a bit of logging, especially a warn! in these cases, as otherwise it might be surprising if information from the SDP simply disappears while parsing.

@sdroege
Copy link
Owner

sdroege commented Jan 1, 2024

The clippy warning should be gone now if you rebase

@sdroege
Copy link
Owner

sdroege commented Feb 14, 2024

@nemosupremo Any updates here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants