Skip to content

Commit 36f1b56

Browse files
authored
Refactor logging and string formatting for ICE (#1668)
Replaced String.Format and concatenation with interpolated strings. Updated all logging to use structured logging with message templates and named parameters. Improved MDNS TLD comparison to use StringComparison.OrdinalIgnoreCase. Changes align with user preferences for structured logging and code clarity.
1 parent 937d4ae commit 36f1b56

2 files changed

Lines changed: 13 additions & 42 deletions

File tree

src/SIPSorcery/net/ICE/RTCIceCandidate.cs

Lines changed: 5 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -248,24 +248,11 @@ public override string ToString()
248248
string candidateStr;
249249
if (protocol == RTCIceProtocol.tcp)
250250
{
251-
candidateStr = String.Format("{0} {1} tcp {2} {3} {4} typ {5} tcptype {6} generation 0",
252-
foundation,
253-
component.GetHashCode(),
254-
priority,
255-
address,
256-
port,
257-
type,
258-
tcpType);
251+
candidateStr = $"{foundation} {component.GetHashCode()} tcp {priority} {address} {port} typ {type} tcptype {tcpType} generation 0";
259252
}
260253
else
261254
{
262-
candidateStr = String.Format("{0} {1} udp {2} {3} {4} typ {5} generation 0",
263-
foundation,
264-
component.GetHashCode(),
265-
priority,
266-
address,
267-
port,
268-
type);
255+
candidateStr = $"{foundation} {component.GetHashCode()} udp {priority} {address} {port} typ {type} generation 0";
269256
}
270257

271258
return candidateStr;
@@ -282,28 +269,11 @@ public override string ToString()
282269
string candidateStr;
283270
if (protocol == RTCIceProtocol.tcp)
284271
{
285-
candidateStr = String.Format("{0} {1} tcp {2} {3} {4} typ {5} tcptype {6} raddr {7} rport {8} generation 0",
286-
foundation,
287-
component.GetHashCode(),
288-
priority,
289-
address,
290-
port,
291-
type,
292-
tcpType,
293-
relAddr,
294-
relatedPort);
272+
candidateStr = $"{foundation} {component.GetHashCode()} tcp {priority} {address} {port} typ {type} tcptype {tcpType} raddr {relAddr} rport {relatedPort} generation 0";
295273
}
296274
else
297275
{
298-
candidateStr = String.Format("{0} {1} udp {2} {3} {4} typ {5} raddr {6} rport {7} generation 0",
299-
foundation,
300-
component.GetHashCode(),
301-
priority,
302-
address,
303-
port,
304-
type,
305-
relAddr,
306-
relatedPort);
276+
candidateStr = $"{foundation} {component.GetHashCode()} udp {priority} {address} {port} typ {type} raddr {relAddr} rport {relatedPort} generation 0";
307277
}
308278

309279
return candidateStr;
@@ -383,7 +353,7 @@ public string toJSON()
383353
sdpMid = sdpMid ?? sdpMLineIndex.ToString(),
384354
sdpMLineIndex = sdpMLineIndex,
385355
usernameFragment = usernameFragment,
386-
candidate = CANDIDATE_PREFIX + ":" + this.ToString()
356+
candidate = $"{CANDIDATE_PREFIX}:{this}"
387357
};
388358

389359
return rtcCandInit.toJSON();

src/SIPSorcery/net/ICE/RtpIceChannel.cs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,7 +1121,7 @@ private void RefreshTurn(Object state)
11211121
}
11221122
catch (Exception excp)
11231123
{
1124-
logger.LogError(excp, "Exception " + nameof(RefreshTurn) + ". {ErrorMessage}", excp);
1124+
logger.LogError(excp, "Exception in {Method}.", nameof(RefreshTurn));
11251125
}
11261126
}
11271127

@@ -1308,12 +1308,12 @@ private async Task UpdateChecklist(RTCIceCandidate localCandidate, RTCIceCandida
13081308
{
13091309
if (localCandidate == null)
13101310
{
1311-
logger.LogError(nameof(UpdateChecklist) + " the local candidate supplied to UpdateChecklist was null.");
1311+
logger.LogError("{Method} the local candidate supplied to UpdateChecklist was null.", nameof(UpdateChecklist));
13121312
return;
13131313
}
13141314
else if (remoteCandidate == null)
13151315
{
1316-
logger.LogError(nameof(UpdateChecklist) + " the remote candidate supplied to UpdateChecklist was null.");
1316+
logger.LogError("{Method} the remote candidate supplied to UpdateChecklist was null.", nameof(UpdateChecklist));
13171317
return;
13181318
}
13191319

@@ -1323,7 +1323,7 @@ private async Task UpdateChecklist(RTCIceCandidate localCandidate, RTCIceCandida
13231323
// Attempt to resolve the remote candidate address.
13241324
if (!IPAddress.TryParse(remoteCandidate.address, out var remoteCandidateIPAddr))
13251325
{
1326-
if (remoteCandidate.address.ToLower().EndsWith(MDNS_TLD))
1326+
if (remoteCandidate.address.EndsWith(MDNS_TLD, StringComparison.OrdinalIgnoreCase))
13271327
{
13281328
var addresses = await ResolveMdnsName(remoteCandidate).ConfigureAwait(false);
13291329
if (addresses.Length == 0)
@@ -1418,7 +1418,7 @@ private async Task UpdateChecklist(RTCIceCandidate localCandidate, RTCIceCandida
14181418
}
14191419
catch (Exception excp)
14201420
{
1421-
logger.LogError(excp, "Exception " + nameof(UpdateChecklist) + ". {ErrorMessage}", excp);
1421+
logger.LogError(excp, "Exception in {Method}.", nameof(UpdateChecklist));
14221422
}
14231423
}
14241424

@@ -1760,7 +1760,7 @@ private void SendSTUNBindingRequest(ChecklistEntry candidatePair, bool setUseCan
17601760
{
17611761
STUNMessage stunRequest = new STUNMessage(STUNMessageTypesEnum.BindingRequest);
17621762
stunRequest.Header.TransactionId = Encoding.ASCII.GetBytes(candidatePair.RequestTransactionID);
1763-
stunRequest.AddUsernameAttribute(RemoteIceUser + ":" + LocalIceUser);
1763+
stunRequest.AddUsernameAttribute($"{RemoteIceUser}:{LocalIceUser}");
17641764
stunRequest.Attributes.Add(new STUNAttribute(STUNAttributeTypesEnum.Priority, BitConverter.GetBytes(candidatePair.LocalPriority)));
17651765

17661766
if (IsController)
@@ -2725,7 +2725,8 @@ private async Task<IPAddress[]> ResolveMdnsName(RTCIceCandidate candidate)
27252725
{
27262726
if (MdnsResolve != null)
27272727
{
2728-
logger.LogWarning("RTP ICE channel has both "+ nameof(MdnsGetAddresses) + " and " + nameof(MdnsGetAddresses) + " set. Only " + nameof(MdnsGetAddresses) + " will be used.");
2728+
logger.LogWarning("RTP ICE channel has both {PrimaryResolver} and {SecondaryResolver} set. Only {SelectedResolver} will be used.",
2729+
nameof(MdnsGetAddresses), nameof(MdnsResolve), nameof(MdnsGetAddresses));
27292730
}
27302731
return await MdnsGetAddresses(candidate.address).ConfigureAwait(false);
27312732
}

0 commit comments

Comments
 (0)