Skip to content

Add hyperlink validation and refactor CreateHyperlink #2994

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

Merged
merged 1 commit into from
Sep 17, 2025
Merged

Add hyperlink validation and refactor CreateHyperlink #2994

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
32 changes: 31 additions & 1 deletion src/MigrationTools.Clients.TfsObjectModel/Tools/TfsWorkItemLinkTool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
/// <param name="targetWorkItemLinkStart">The target work item to receive the links</param>
/// <returns>The number of links processed</returns>
/// <exception cref="ArgumentNullException">Thrown when source or target work items are null</exception>
public int Enrich(TfsProcessor processor, WorkItemData sourceWorkItemLinkStart, WorkItemData targetWorkItemLinkStart)

Check warning on line 43 in src/MigrationTools.Clients.TfsObjectModel/Tools/TfsWorkItemLinkTool.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package 

Refactor this method to reduce its Cognitive Complexity from 26 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
if (sourceWorkItemLinkStart is null)
{
Expand All @@ -58,7 +58,7 @@

if (ShouldCopyLinks(sourceWorkItemLinkStart, targetWorkItemLinkStart))
{
Log.LogTrace("Links = '{@sourceWorkItemLinkStartLinks}", sourceWorkItemLinkStart.Links);

Check warning on line 61 in src/MigrationTools.Clients.TfsObjectModel/Tools/TfsWorkItemLinkTool.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package 

Use PascalCase for named placeholders. (https://rules.sonarsource.com/csharp/RSPEC-6678)
foreach (Link item in sourceWorkItemLinkStart.ToWorkItem().Links)
{
try
Expand Down Expand Up @@ -146,7 +146,7 @@

// DevOps doesn't seem to take impersonation very nicely here - as of 13.05.22 the following line would get you an error:
// System.FormatException: The string 'Microsoft.TeamFoundation.WorkItemTracking.Common.ServerDefaultFieldValue' is not a valid AllXsd value.
// target.ToWorkItem().Fields["System.ModifiedBy"].Value = "Migration";

Check warning on line 149 in src/MigrationTools.Clients.TfsObjectModel/Tools/TfsWorkItemLinkTool.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package 

Remove this commented out code. (https://rules.sonarsource.com/csharp/RSPEC-125)
}
}

Expand Down Expand Up @@ -210,7 +210,7 @@

// DevOps doesn't seem to take impersonation very nicely here - as of 13.05.22 the following line would get you an error:
// System.FormatException: The string 'Microsoft.TeamFoundation.WorkItemTracking.Common.ServerDefaultFieldValue' is not a valid AllXsd value.
// target.ToWorkItem().Fields["System.ModifiedBy"].Value = "Migration";

Check warning on line 213 in src/MigrationTools.Clients.TfsObjectModel/Tools/TfsWorkItemLinkTool.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package 

Remove this commented out code. (https://rules.sonarsource.com/csharp/RSPEC-125)
if (Options.SaveAfterEachLinkIsAdded)
{
try
Expand Down Expand Up @@ -324,7 +324,7 @@
var potentialParentConflictLink = ( // TF201036: You cannot add a Child link between work items xxx and xxx because a work item can have only one Parent link.
from Link l in wiTargetR.ToWorkItem().Links
where l is RelatedLink
&& ((RelatedLink)l).LinkTypeEnd.ImmutableName == "System.LinkTypes.Hierarchy-Reverse"

Check warning on line 327 in src/MigrationTools.Clients.TfsObjectModel/Tools/TfsWorkItemLinkTool.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package 

Define a constant instead of using this literal 'System.LinkTypes.Hierarchy-Reverse' 4 times. (https://rules.sonarsource.com/csharp/RSPEC-1192)
select (RelatedLink)l).SingleOrDefault();
if (potentialParentConflictLink != null)
{
Expand All @@ -336,7 +336,7 @@

// DevOps doesn't seem to take impersonation very nicely here - as of 13.05.22 the following line would get you an error:
// System.FormatException: The string 'Microsoft.TeamFoundation.WorkItemTracking.Common.ServerDefaultFieldValue' is not a valid AllXsd value.
// wiTargetR.ToWorkItem().Fields["System.ModifiedBy"].Value = "Migration";

Check warning on line 339 in src/MigrationTools.Clients.TfsObjectModel/Tools/TfsWorkItemLinkTool.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package 

Remove this commented out code. (https://rules.sonarsource.com/csharp/RSPEC-125)
wiTargetR.SaveToAzureDevOps();
}
else
Expand All @@ -357,7 +357,7 @@

// DevOps doesn't seem to take impersonation very nicely here - as of 13.05.22 the following line would get you an error:
// System.FormatException: The string 'Microsoft.TeamFoundation.WorkItemTracking.Common.ServerDefaultFieldValue' is not a valid AllXsd value.
// wiTargetL.ToWorkItem().Fields["System.ModifiedBy"].Value = "Migration";

Check warning on line 360 in src/MigrationTools.Clients.TfsObjectModel/Tools/TfsWorkItemLinkTool.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package 

Remove this commented out code. (https://rules.sonarsource.com/csharp/RSPEC-125)
if (Options.SaveAfterEachLinkIsAdded)
{
wiTargetL.SaveToAzureDevOps();
Expand Down Expand Up @@ -425,12 +425,17 @@

private void CreateHyperlink(Hyperlink sourceLink, WorkItemData target)
{
var sourceLinkAbsoluteUri = GetAbsoluteUri(sourceLink);
string sourceLinkAbsoluteUri = GetAbsoluteUri(sourceLink);
if (string.IsNullOrEmpty(sourceLinkAbsoluteUri))
{
Log.LogWarning(" [SKIP] Unable to create a hyperlink to [{0}]", sourceLink.Location);
return;
}
if (!IsValidHyperlink(sourceLinkAbsoluteUri))
{
Log.LogWarning(" [SKIP] Unable to create a hyperlink to [{0}] <-- protocall no longer allowed or url format is invalid", sourceLink.Location);
return;
}

var exist = (from hyperlink in target.ToWorkItem().Links.OfType<Hyperlink>()
let absoluteUri = GetAbsoluteUri(hyperlink)
Expand All @@ -451,13 +456,15 @@

// DevOps doesn't seem to take impersonation very nicely here - as of 13.05.22 the following line would get you an error:
// System.FormatException: The string 'Microsoft.TeamFoundation.WorkItemTracking.Common.ServerDefaultFieldValue' is not a valid AllXsd value.
// target.ToWorkItem().Fields["System.ModifiedBy"].Value = "Migration";

Check warning on line 459 in src/MigrationTools.Clients.TfsObjectModel/Tools/TfsWorkItemLinkTool.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package 

Remove this commented out code. (https://rules.sonarsource.com/csharp/RSPEC-125)
if (Options.SaveAfterEachLinkIsAdded)
{
target.SaveToAzureDevOps();
}
}



private string GetAbsoluteUri(Hyperlink hyperlink)
{
try
Expand Down Expand Up @@ -489,5 +496,28 @@
return item is Hyperlink;
}

private bool IsValidHyperlink(string sourceLinkAbsoluteUri)
{
if (string.IsNullOrEmpty(sourceLinkAbsoluteUri))
{
return false;
}

try
{
var uri = new Uri(sourceLinkAbsoluteUri);
var allowedSchemes = new[]
{
"http", "https", "ftp", "gopher", "mailto", "news", "telnet", "wais",
"vstfs", "tfs", "alm", "mtm", "mtms", "mtr", "mtrs", "mfbclient", "mfbclients",
"test-runner", "x-mvwit", "onenote", "codeflow", "file", "tel", "skype"
};
return allowedSchemes.Contains(uri.Scheme.ToLowerInvariant());
}
catch (UriFormatException)
{
return false;
}
}
}
}
Loading