Add SDP parsing regression tests for malformed crypto and rtpmap lines#1658
Merged
Conversation
Adds regression tests asserting the desired SDP parser behaviour for recognised-but-invalid lines: - a crypto line with a whitespace-only value -> TryParse returns false - a crypto line missing the crypto-suite field -> TryParse returns false - a crypto line missing key parameters -> TryParse returns false - an rtpmap attribute with a non-numeric payload ID is dropped (not round-tripped): it produces no media format entry, does not corrupt the valid formats, and is not re-emitted by ToString(). These encode the intended behaviour delivered by #1655. They are expected to fail against the current master parser (which throws an IndexOutOfRangeException on the malformed crypto lines and round-trips the invalid rtpmap line) and to pass once #1655 lands. Tests only; no production code changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| { | ||
| private const string CRLF = "\r\n"; | ||
|
|
||
| private Microsoft.Extensions.Logging.ILogger logger = null; |
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds regression tests pinning the desired SDP parser behaviour for recognised-but-invalid lines — lines whose attribute type the parser understands but whose content is malformed. Tests only; no production code changes.
CryptoLineWithWhitespaceValueReturnsFalsea=crypto:TryParsereturnsfalse(no throw)CryptoLineWithMissingCryptoSuiteReturnsFalsea=crypto:1TryParsereturnsfalse(no throw)CryptoLineWithMissingKeyParamsReturnsFalsea=crypto:1 AES_CM_128_HMAC_SHA1_80TryParsereturnsfalseNonNumericRtpmapIdIsDroppedNotRoundTrippeda=rtpmap:xyz nonsense/1ToString()Behavioural principle being encoded
rtpmap/cryptobut the content is bad) → drop it. RTP payload types are always numeric, so a non-numeric rtpmap id is known-bad and should not survive a round-trip.masterThese tests encode the behaviour delivered by #1655 (Span-based SDP rewrite). Against the current
masterparser they fail, by design:IndexOutOfRangeException(latent crash inSDPSecurityDescription.TryParse, which indexessCryptoParts[0]/[1]without a length guard);rtpmapline is round-tripped as an extra attribute instead of being dropped.Verified: 4/4 pass on the #1655 branch; on
master3 fail / 1 passes. They will go green once #1655 merges. The crash fixes themselves are coming in #1655, so this PR intentionally carries no production changes.🤖 Generated with Claude Code