AmB2BSession::updateLocalBody: drop empty SDP part via deletePart()#548
Merged
Conversation
The zero-length SDP short-circuit added in 7c5374c called AmMimeBody::clear(), which doesn't exist on the class - clearParts(), clearPart() and clearPayload() are private and there is no public clear() - so the build failed with: AmB2BSession.cpp:434:10: error: 'class AmMimeBody' has no member named 'clear' The intent (per the original commit message) is to drop the empty application/sdp part from the surrounding multipart body. The public API for that is AmMimeBody::deletePart(content_type) on the parent body, which removes the matching sub-part and converts back to single- part if only one remains. Switch to it.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a build break in AmB2BSession::updateLocalBody() by replacing a call to a non-existent AmMimeBody::clear() with AmMimeBody::deletePart(SIP_APPLICATION_SDP) to drop a zero-length SDP body part.
Changes:
- Replace
sdp->clear()withbody.deletePart(SIP_APPLICATION_SDP)when anapplication/sdppart has zero payload.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if (!sdp->getLen()) { | ||
| sdp->clear(); | ||
| body.deletePart(SIP_APPLICATION_SDP); |
|
|
||
| if (!sdp->getLen()) { | ||
| sdp->clear(); | ||
| body.deletePart(SIP_APPLICATION_SDP); |
UACAuth.cpp:295 wraps MD5Update with:
void w_MD5Update(MD5_CTX *ctx, const string& s) {
MD5Update(ctx, (const unsigned char *)s.data(), s.length());
}
string::data() returns a const pointer, but the legacy MD5Update
declaration in md5.h takes a non-const unsigned char *, so on
toolchains that don't silently accept the const-stripping cast
(e.g. clang on macOS arm64 used by the CI runner) the build fails:
no matching function for call to 'MD5Update'
candidate function not viable: 2nd argument ('const unsigned char *')
would lose const qualifier
void MD5Update(MD5_CTX *, unsigned char *, unsigned int);
MD5Update only reads from the input buffer (it copies bytes into the
context's own buffer via MD5_memcpy and feeds them to MD5Transform);
nothing in the chain mutates caller memory. Widen the parameter to
const unsigned char *. The internal MD5Transform call needs a
matching cast since MD5Transform's legacy signature still takes a
non-const block pointer; the cast is sound because MD5Transform
likewise only reads from the block.
All existing callers (AmUtils, UACAuth, the recursive MD5Update calls
inside MD5Final) compile unchanged: a non-const pointer converts
implicitly to const, and the const-correct call site that was failing
on macOS now matches.
Commit 7c5374c backported yeti-switch/sems@25aae386 ("AmB2BSession: updateLocalBody: fix processing of the zero-length SDP body"), which calls sdp->clear() on the empty application/sdp part: if (!sdp->getLen()) { sdp->clear(); return; } The call site landed but the upstream public AmMimeBody::clear() method (and the AmContentType::clear() it delegates to) were never brought across, so the build broke with: AmB2BSession.cpp:434:10: error: 'class AmMimeBody' has no member named 'clear' c46584a worked around this by switching the call to body.deletePart(SIP_APPLICATION_SDP). That compiles and is safe, but its semantics differ from upstream: - Single-part body whose own content-type is application/sdp: deletePart() returns -1 (body is not multipart), so the empty body lingers untouched. Upstream clear() wipes it (payload, parts, content-type, length) into a fully empty state. - Multipart body containing an empty SDP sub-part: deletePart() removes the sub-part from the parent's parts list (and collapses to single-part if exactly one remains). Upstream clear() leaves the sub-part in place but resets its payload and content-type so it serialises as an empty container. The author's intent — and the explicit "Backport of yeti-switch/sems@25aae386" attribution on 7c5374c — point at matching upstream behaviour, not at substituting a different cleanup primitive. Backport the missing methods literally from upstream: void AmContentType::clear() { type.clear(); subtype.clear(); clearParams(); } void AmMimeBody::clear() { clearPayload(); clearParts(); content_len = 0; ct.clear(); } and revert AmB2BSession::updateLocalBody to call sdp->clear() as the original commit (and its upstream) wrote.
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
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.
The zero-length SDP short-circuit added in 7c5374c calls
AmMimeBody::clear(), which doesn't exist on the class —clearParts(),clearPart()andclearPayload()are all private and there is no publicclear()— so the build fails with:The intent (per the original commit message) is to drop the empty
application/sdppart from the surrounding multipart body. The public API for that isAmMimeBody::deletePart(content_type)on the parent body, which removes the matching sub-part and converts back to single-part if only one remains. Switch to it.Summary
sdp->clear()call insideAmB2BSession::updateLocalBodywithbody.deletePart(SIP_APPLICATION_SDP).Test plan
make sems_tests && ./core/sems_testssucceeds (the previous failure was the compile error itself).application/sdppart has zero payload no longer carries the empty part forward (matches the behaviour described in 7c5374c).Generated by Claude Code