From b6b14c66a7a7a3ae8f1a85216a586d0683cfaa58 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Fri, 1 Mar 2019 09:15:03 -0800 Subject: [PATCH] Fix minor Activity W3C format issues (dotnet/corefx#35650) * Fix minor W3C Activity format issues * undo csproj changes * review nits * review: parentSpanId invalid * add security safe critical attributes, fix tests Commit migrated from https://github.com/dotnet/corefx/commit/ba5d02d92ecdf5ee902f8e4b348bc0ffd2a34a96 --- .../src/System/Diagnostics/Activity.cs | 47 ++++++++++++++--- .../tests/ActivityTests.cs | 60 ++++++++++++++++++---- 2 files changed, 89 insertions(+), 18 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs index a320a8a..9310688 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs @@ -458,6 +458,9 @@ namespace System.Diagnostics /// public ref ActivitySpanId SpanId { +#if ALLOW_PARTIALLY_TRUSTED_CALLERS + [System.Security.SecuritySafeCriticalAttribute] +#endif get { if (!_spanIdSet) @@ -485,6 +488,9 @@ namespace System.Diagnostics /// public ref ActivityTraceId TraceId { +#if ALLOW_PARTIALLY_TRUSTED_CALLERS + [System.Security.SecuritySafeCriticalAttribute] +#endif get { if (!_traceIdSet) @@ -505,12 +511,27 @@ namespace System.Diagnostics /// public ref ActivitySpanId ParentSpanId { +#if ALLOW_PARTIALLY_TRUSTED_CALLERS + [System.Security.SecuritySafeCriticalAttribute] +#endif get { - if (!_parentSpanIdSet && _parentId != null && IsW3CId(_parentId)) + if (!_parentSpanIdSet) { - _parentSpanId = new ActivitySpanId(_parentId.AsSpan(36, 16)); - _parentSpanIdSet = true; + if (_parentId != null && IsW3CId(_parentId)) + { + try + { + _parentSpanId = new ActivitySpanId(_parentId.AsSpan(36, 16)); + } + catch { } + _parentSpanIdSet = true; + } + else if (Parent != null && Parent.IdFormat == ActivityIdFormat.W3C) + { + _parentSpanId = Parent.SpanId; + _parentSpanIdSet = true; + } } return ref _parentSpanId; } @@ -584,11 +605,25 @@ namespace System.Diagnostics if (!_traceIdSet) { if (Parent != null && Parent.IdFormat == ActivityIdFormat.W3C) + { _traceId = Parent.TraceId; + } else if (_parentId != null && IsW3CId(_parentId)) - _traceId = new ActivityTraceId(_parentId.AsSpan(3, 32)); + { + try + { + _traceId = new ActivityTraceId(_parentId.AsSpan(3, 32)); + } + catch + { + _traceId = ActivityTraceId.NewTraceId(); + } + } else + { _traceId = ActivityTraceId.NewTraceId(); + } + _traceIdSet = true; } // Create a new SpanID. @@ -944,8 +979,6 @@ namespace System.Diagnostics { if ('0' <= c && c <= '9') return (byte)(c - '0'); - if ('A' <= c && c <= 'F') - return (byte)(c - ('A' - 10)); if ('a' <= c && c <= 'f') return (byte)(c - ('a' - 10)); throw new ArgumentOutOfRangeException("idData"); @@ -955,7 +988,7 @@ namespace System.Diagnostics val &= 0xF; if (val <= 9) return (char)('0' + val); - return (char)(('A' - 10) + val); + return (char)(('a' - 10) + val); } #endregion diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs index 454e355..f388698 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs @@ -288,12 +288,12 @@ namespace System.Diagnostics.Tests return false; if (id[52] != '-') return false; - return Regex.IsMatch(id, "^[0-9A-Fa-f][0-9A-Fa-f]-[0-9A-Fa-f]*-[0-9A-Fa-f]*-[0-9A-Fa-f][0-9A-Fa-f]$"); + return Regex.IsMatch(id, "^[0-9a-f][0-9a-f]-[0-9a-f]*-[0-9a-f]*-[0-9a-f][0-9a-f]$"); } - public static bool IsHex(string s) + public static bool IsLowerCaseHex(string s) { - return Regex.IsMatch(s, "^[0-9A-Fa-f]*$"); + return Regex.IsMatch(s, "^[0-9a-f]*$"); } /****** ActivityTraceId tests *****/ @@ -318,7 +318,7 @@ namespace System.Diagnostics.Tests // NewActivityTraceId ActivityTraceId newId1 = ActivityTraceId.NewTraceId(); - Assert.True(IsHex(newId1.AsHexString)); + Assert.True(IsLowerCaseHex(newId1.AsHexString)); Assert.Equal(32, newId1.AsHexString.Length); ActivityTraceId newId2 = ActivityTraceId.NewTraceId(); @@ -369,7 +369,7 @@ namespace System.Diagnostics.Tests Assert.Equal(newId2.GetHashCode(), newId2Clone.GetHashCode()); // String constructor and AsHexString. - string idStr = "0123456789ABCDEF0123456789ABCDEF"; + string idStr = "0123456789abcdef0123456789abcdef"; ActivityTraceId id = new ActivityTraceId(idStr.AsSpan()); Assert.Equal(idStr, id.AsHexString); @@ -404,7 +404,7 @@ namespace System.Diagnostics.Tests // NewActivitySpanId ActivitySpanId newId1 = ActivitySpanId.NewSpanId(); - Assert.True(IsHex(newId1.AsHexString)); + Assert.True(IsLowerCaseHex(newId1.AsHexString)); Assert.Equal(16, newId1.AsHexString.Length); ActivitySpanId newId2 = ActivitySpanId.NewSpanId(); @@ -455,7 +455,7 @@ namespace System.Diagnostics.Tests Assert.Equal(newId2.GetHashCode(), newId2Clone.GetHashCode()); // String constructor and AsHexString. - string idStr = "0123456789ABCDEF"; + string idStr = "0123456789abcdef"; ActivitySpanId id = new ActivitySpanId(idStr.AsSpan()); Assert.Equal(idStr, id.AsHexString); @@ -482,16 +482,17 @@ namespace System.Diagnostics.Tests Assert.Equal(ActivityIdFormat.Hierarchical, activity.IdFormat); activity.Stop(); - // Set the parent to something that is WC3 by string + // Set the parent to something that is W3C by string activity = new Activity("activity2"); - activity.SetParentId("00-0123456789ABCDEF0123456789ABCDEF-0123456789ABCDEF-01"); + activity.SetParentId("00-0123456789abcdef0123456789abcdef-0123456789abcdef-01"); activity.Start(); Assert.Equal(ActivityIdFormat.W3C, activity.IdFormat); - Assert.Equal("0123456789ABCDEF0123456789ABCDEF", activity.TraceId.AsHexString); + Assert.Equal("0123456789abcdef0123456789abcdef", activity.TraceId.AsHexString); + Assert.Equal("0123456789abcdef", activity.ParentSpanId.AsHexString); Assert.True(IdIsW3CFormat(activity.Id)); activity.Stop(); - // Set the parent to something that is WC3 byt using ActivityTraceId,ActivitySpanId version of SetParentId. + // Set the parent to something that is W3C but using ActivityTraceId,ActivitySpanId version of SetParentId. activity = new Activity("activity3"); ActivityTraceId activityTraceId = ActivityTraceId.NewTraceId(); activity.SetParentId(activityTraceId, ActivitySpanId.NewSpanId()); @@ -564,6 +565,43 @@ namespace System.Diagnostics.Tests activity.Stop(); parent.Stop(); + + // Upper-case ids are not supported + activity = new Activity("activity8"); + activity.SetParentId("00-0123456789ABCDEF0123456789ABCDEF-0123456789ABCDEF-01"); + activity.Start(); + Assert.Equal(ActivityIdFormat.W3C, activity.IdFormat); + Assert.True(IdIsW3CFormat(activity.Id)); + activity.Stop(); + + // non hex chars are not supported in traceId + activity = new Activity("activity9"); + activity.SetParentId("00-xyz3456789abcdef0123456789abcdef-0123456789abcdef-01"); + activity.Start(); + Assert.Equal(ActivityIdFormat.W3C, activity.IdFormat); + Assert.True(IdIsW3CFormat(activity.Id)); + activity.Stop(); + + // non hex chars are not supported in parentSpanId + activity = new Activity("activity10"); + activity.SetParentId("00-0123456789abcdef0123456789abcdef-x123456789abcdef-01"); + activity.Start(); + Assert.Equal(ActivityIdFormat.W3C, activity.IdFormat); + Assert.True(IdIsW3CFormat(activity.Id)); + Assert.Equal("0000000000000000", activity.ParentSpanId.AsHexString); + Assert.Equal("0123456789abcdef0123456789abcdef", activity.TraceId.AsHexString); + activity.Stop(); + + // ParentSpanId from parent Activity + Activity.DefaultIdFormat = ActivityIdFormat.W3C; + Activity.ForceDefaultIdFormat = true; + + parent = new Activity("parent").Start(); + activity = new Activity("parent").Start(); + Assert.Equal(parent.SpanId.AsHexString, activity.ParentSpanId.AsHexString); + + activity.Stop(); + parent.Stop(); } finally { -- 2.7.4