From dfe2456ab7363f33e39918c34e0f5ef87fb0fd2c Mon Sep 17 00:00:00 2001 From: Brad Ebinger Date: Fri, 2 Apr 2021 19:14:05 +0000 Subject: [PATCH] Validate SipMessages have a correct Via branch parameter SIP messages are required to have a transaction ID, which is based on the Via header's unique branch parameter. For processing, we require this parameter to be set in order to send state updates about the message. Also, as per council guidelines in b/184151053, throw a NPE instaed of IllegalArgumentException for null params. Fixes: 181087377 Test: atest CtsTelephonyTestCases Change-Id: Ic4a637e780d04e9ac5d8ba01d7ba6dfae92307b1 --- core/api/system-current.txt | 2 +- .../java/android/telephony/ims/SipMessage.java | 18 +++++++++++------- .../ims/aidl/SipDelegateAidlWrapper.java | 7 ------- .../aidl/SipDelegateConnectionAidlWrapper.java | 9 +-------- 4 files changed, 13 insertions(+), 23 deletions(-) diff --git a/core/api/system-current.txt b/core/api/system-current.txt index 3f11b722306a3..e6624cdb865a4 100644 --- a/core/api/system-current.txt +++ b/core/api/system-current.txt @@ -11988,7 +11988,7 @@ package android.telephony.ims { method @NonNull public byte[] getEncodedMessage(); method @NonNull public String getHeaderSection(); method @NonNull public String getStartLine(); - method @Nullable public String getViaBranchParameter(); + method @NonNull public String getViaBranchParameter(); method public void writeToParcel(@NonNull android.os.Parcel, int); field @NonNull public static final android.os.Parcelable.Creator CREATOR; } diff --git a/telephony/java/android/telephony/ims/SipMessage.java b/telephony/java/android/telephony/ims/SipMessage.java index ad6d73c399628..b5295637d4dd6 100644 --- a/telephony/java/android/telephony/ims/SipMessage.java +++ b/telephony/java/android/telephony/ims/SipMessage.java @@ -24,6 +24,7 @@ import android.annotation.SystemApi; import android.os.Build; import android.os.Parcel; import android.os.Parcelable; +import android.text.TextUtils; import com.android.internal.telephony.SipMessageParsingUtils; @@ -60,14 +61,19 @@ public final class SipMessage implements Parcelable { */ public SipMessage(@NonNull String startLine, @NonNull String headerSection, @NonNull byte[] content) { - if (startLine == null || headerSection == null || content == null) { - throw new IllegalArgumentException("One or more null parameters entered"); - } + Objects.requireNonNull(startLine, "Required parameter is null: startLine"); + Objects.requireNonNull(headerSection, "Required parameter is null: headerSection"); + Objects.requireNonNull(content, "Required parameter is null: content"); + mStartLine = startLine; mHeaderSection = headerSection; mContent = content; mViaBranchParam = SipMessageParsingUtils.getTransactionId(mHeaderSection); + if (TextUtils.isEmpty(mViaBranchParam)) { + throw new IllegalArgumentException("header section MUST contain a branch parameter " + + "inside of the Via header."); + } mCallIdParam = SipMessageParsingUtils.getCallId(mHeaderSection); } @@ -107,11 +113,9 @@ public final class SipMessage implements Parcelable { /** * @return the branch parameter enclosed in the Via header key's value. See RFC 3261 section - * 20.42 for more information on the Via header. If {@code null}, then there was either no - * Via parameter found in this SIP message's headers or no branch parameter found in the - * Via header. + * 20.42 for more information on the Via header. */ - public @Nullable String getViaBranchParameter() { + public @NonNull String getViaBranchParameter() { return mViaBranchParam; } diff --git a/telephony/java/android/telephony/ims/aidl/SipDelegateAidlWrapper.java b/telephony/java/android/telephony/ims/aidl/SipDelegateAidlWrapper.java index 739946be2e5bc..5c9ec53d713be 100644 --- a/telephony/java/android/telephony/ims/aidl/SipDelegateAidlWrapper.java +++ b/telephony/java/android/telephony/ims/aidl/SipDelegateAidlWrapper.java @@ -28,8 +28,6 @@ import android.telephony.ims.SipDelegateImsConfiguration; import android.telephony.ims.SipDelegateManager; import android.telephony.ims.SipMessage; import android.telephony.ims.stub.SipDelegate; -import android.text.TextUtils; -import android.util.Log; import java.util.ArrayList; import java.util.Set; @@ -187,11 +185,6 @@ public class SipDelegateAidlWrapper implements DelegateStateCallback, DelegateMe private void notifyLocalMessageFailedToBeReceived(SipMessage m, int reason) { String transactionId = m.getViaBranchParameter(); - if (TextUtils.isEmpty(transactionId)) { - Log.w(LOG_TAG, "failure to parse SipMessage."); - throw new IllegalArgumentException("Malformed SipMessage, can not determine " - + "transaction ID."); - } SipDelegate d = mDelegate; if (d != null) { mExecutor.execute(() -> d.notifyMessageReceiveError(transactionId, reason)); diff --git a/telephony/java/android/telephony/ims/aidl/SipDelegateConnectionAidlWrapper.java b/telephony/java/android/telephony/ims/aidl/SipDelegateConnectionAidlWrapper.java index 3cd27264295cf..ad02fe55902f1 100644 --- a/telephony/java/android/telephony/ims/aidl/SipDelegateConnectionAidlWrapper.java +++ b/telephony/java/android/telephony/ims/aidl/SipDelegateConnectionAidlWrapper.java @@ -28,7 +28,6 @@ import android.telephony.ims.SipMessage; import android.telephony.ims.stub.DelegateConnectionMessageCallback; import android.telephony.ims.stub.DelegateConnectionStateCallback; import android.telephony.ims.stub.SipDelegate; -import android.text.TextUtils; import android.util.ArraySet; import android.util.Log; @@ -267,12 +266,6 @@ public class SipDelegateConnectionAidlWrapper implements SipDelegateConnection, private void notifyLocalMessageFailedToSend(SipMessage m, int reason) { String transactionId = m.getViaBranchParameter(); - if (TextUtils.isEmpty(transactionId)) { - Log.w(LOG_TAG, "sendMessage detected a malformed SipMessage and can not get a " - + "transaction ID."); - throw new IllegalArgumentException("Could not send SipMessage due to malformed header"); - } - mExecutor.execute(() -> - mMessageCallback.onMessageSendFailure(transactionId, reason)); + mExecutor.execute(() -> mMessageCallback.onMessageSendFailure(transactionId, reason)); } }