Skip to content

Change the definition of enum SIPExtensions to allow using as a bitfield #1323

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Change the definition of enum SIPExtensions to allow using as a bitfield #1323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 50 additions & 34 deletions src/core/SIP/SIPConstants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,15 +603,16 @@ public static string SIPURIParameterUnescape(string escapedString)

///<summary>
/// List of SIP extensions to RFC3262.
/// </summary>
/// </summary>
[Flags]
public enum SIPExtensions
{
None = 0,
Prack = 1, // Reliable provisional responses as per RFC3262.
NoReferSub = 2, // No subscription for REFERs as per RFC4488.
Replaces = 3,
SipRec = 4,
MultipleRefer = 5,
Replaces = 4,
SipRec = 8,
MultipleRefer = 16,
}

/// <summary>
Expand All @@ -627,51 +628,66 @@ public static class SIPExtensionHeaders
public const string MULTIPLE_REFER = "multiple-refer";

/// <summary>
/// Parses a string containing a list of SIP extensions into a list of extensions that this library
/// Parses a string containing a list of SIP extensions into a bitfield of extensions that this library
/// understands.
/// </summary>
/// <param name="extensionList">The string containing the list of extensions to parse.</param>
/// <param name="unknownExtensions">A comma separated list of the unsupported extensions.</param>
/// <returns>A list of extensions that were understood and a boolean indicating whether any unknown extensions were present.</returns>
public static List<SIPExtensions> ParseSIPExtensions(string extensionList, out string unknownExtensions)
/// <returns>A <see cref="SIPExtensions"/> bitfield of SIP extensions that were understood.</returns>
public static SIPExtensions ParseSIPExtensions(string extensionList, out string unknownExtensions)
{
List<SIPExtensions> knownExtensions = new List<SIPExtensions>();
SIPExtensions knownExtensions = 0;
var unknownExtensionsStringBuilder = new StringBuilder();
unknownExtensions = null;

if (String.IsNullOrEmpty(extensionList) == false)
if (String.IsNullOrEmpty(extensionList))
{
string[] extensions = extensionList.Trim().Split(',');
return knownExtensions;
}

foreach (string extension in extensions)
var extensions = extensionList.Split(',');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allocates a string[] and several strings.

Consider using StringTokenizer


foreach (var extension in extensions)
{
if (!String.IsNullOrEmpty(extension))
{
if (String.IsNullOrEmpty(extension) == false)
var trimmedExtension = extension.Trim().ToLower();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is potentially creating 2 strings and using current culture dependent operations.

I know the switch is convenient, but not when dealing with strings.

switch (trimmedExtension)
{
string trimmedExtension = extension.Trim().ToLower();
switch (trimmedExtension)
{
case PRACK:
knownExtensions.Add(SIPExtensions.Prack);
break;
case NO_REFER_SUB:
knownExtensions.Add(SIPExtensions.NoReferSub);
break;
case REPLACES:
knownExtensions.Add(SIPExtensions.Replaces);
break;
case SIPREC:
knownExtensions.Add(SIPExtensions.SipRec);
break;
case MULTIPLE_REFER:
knownExtensions.Add(SIPExtensions.MultipleRefer);
break;
default:
unknownExtensions += (unknownExtensions != null) ? $",{extension.Trim()}" : extension.Trim();
break;
}
case PRACK:
knownExtensions |= SIPExtensions.Prack;
break;

case NO_REFER_SUB:
knownExtensions |= SIPExtensions.NoReferSub;
break;

case REPLACES:
knownExtensions |= SIPExtensions.Replaces;
break;

case SIPREC:
knownExtensions |= SIPExtensions.SipRec;
break;

case MULTIPLE_REFER:
knownExtensions |= SIPExtensions.MultipleRefer;
break;

default:
unknownExtensionsStringBuilder.Append(extension.Trim());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trimming again?

unknownExtensionsStringBuilder.Append(',');
break;
}
}
}

if (unknownExtensionsStringBuilder.Length > 0)
{
unknownExtensionsStringBuilder.Length--; // Get rid of the last comma
unknownExtensions = unknownExtensionsStringBuilder.ToString();
}

return knownExtensions;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/core/SIP/SIPHeader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1516,9 +1516,9 @@ public class SIPHeader

public List<string> UnknownHeaders = new List<string>(); // Holds any unrecognised headers.

public List<SIPExtensions> RequiredExtensions = new List<SIPExtensions>();
public SIPExtensions RequiredExtensions = 0;
public SIPExtensions SupportedExtensions = 0;
public string UnknownRequireExtension = null;
public List<SIPExtensions> SupportedExtensions = new List<SIPExtensions>();

public bool HasAuthenticationHeader => AuthenticationHeaders.Count > 0;

Expand Down
4 changes: 2 additions & 2 deletions src/core/SIPTransactions/SIPTransaction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,8 @@ protected SIPTransaction(
m_sentBy = transactionRequest.Header.Vias.TopViaHeader.ContactAddress;
OutboundProxy = outboundProxy;

if (transactionRequest.Header.RequiredExtensions.Contains(SIPExtensions.Prack) ||
transactionRequest.Header.SupportedExtensions.Contains(SIPExtensions.Prack))
if (transactionRequest.Header.RequiredExtensions.HasFlag(SIPExtensions.Prack) ||
transactionRequest.Header.SupportedExtensions.HasFlag(SIPExtensions.Prack))
{
PrackSupported = true;
}
Expand Down
4 changes: 2 additions & 2 deletions test/unit/core/SIP/SIPHeaderUnitTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,8 +777,8 @@ public void ParseRequireAndSupportedExtensionsTest()
string[] headersCollection = Regex.Split(inviteHeaders, "\r\n");
SIPHeader sipHeader = SIPHeader.ParseSIPHeaders(headersCollection);

Assert.True(sipHeader.RequiredExtensions.Contains(SIPExtensions.Prack), "The required header extensions was missing Prack.");
Assert.True(sipHeader.SupportedExtensions.Contains(SIPExtensions.Prack), "The supported header extensions was missing Prack.");
Assert.True(sipHeader.RequiredExtensions.HasFlag(SIPExtensions.Prack), "The required header extensions was missing Prack.");
Assert.True(sipHeader.SupportedExtensions.HasFlag(SIPExtensions.Prack), "The supported header extensions was missing Prack.");
Assert.True(sipHeader.UnknownRequireExtension != null, "The had unknown required header extension was not correctly set.");

logger.LogDebug("---------------------------------------------------");
Expand Down