From 513f39f62390e60ea6b74c519ae3904715937afa Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Thu, 11 Apr 2024 11:18:00 +0200 Subject: [PATCH] Prevent template circular references (#15907) * Added a simpel circular reference check to templates * Upgraded detecting to all levels * Rename method and add comment to clarify why we have 2 similar methods * Apply suggestions from code review Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> --------- Co-authored-by: Sven Geusens Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> --- .../Template/TemplateControllerBase.cs | 4 ++ .../TemplateOperationStatus.cs | 3 +- src/Umbraco.Core/Services/TemplateService.cs | 46 +++++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Template/TemplateControllerBase.cs b/src/Umbraco.Cms.Api.Management/Controllers/Template/TemplateControllerBase.cs index 568143c80f..ff16139f9c 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Template/TemplateControllerBase.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Template/TemplateControllerBase.cs @@ -30,6 +30,10 @@ public class TemplateControllerBase : ManagementApiControllerBase .WithTitle("Duplicate alias") .WithDetail("A template with that alias already exists.") .Build()), + TemplateOperationStatus.CircularMasterTemplateReference => BadRequest(problemDetailsBuilder + .WithTitle("Invalid master template") + .WithDetail("The master template referenced in the template leads to a circular reference.") + .Build()), _ => StatusCode(StatusCodes.Status500InternalServerError, problemDetailsBuilder .WithTitle("Unknown template operation status.") .Build()), diff --git a/src/Umbraco.Core/Services/OperationStatus/TemplateOperationStatus.cs b/src/Umbraco.Core/Services/OperationStatus/TemplateOperationStatus.cs index 0eede51b77..d7de5441e7 100644 --- a/src/Umbraco.Core/Services/OperationStatus/TemplateOperationStatus.cs +++ b/src/Umbraco.Core/Services/OperationStatus/TemplateOperationStatus.cs @@ -7,5 +7,6 @@ public enum TemplateOperationStatus InvalidAlias, DuplicateAlias, TemplateNotFound, - MasterTemplateNotFound + MasterTemplateNotFound, + CircularMasterTemplateReference } diff --git a/src/Umbraco.Core/Services/TemplateService.cs b/src/Umbraco.Core/Services/TemplateService.cs index 675f5e5cd8..7f13471662 100644 --- a/src/Umbraco.Core/Services/TemplateService.cs +++ b/src/Umbraco.Core/Services/TemplateService.cs @@ -242,6 +242,14 @@ public class TemplateService : RepositoryService, ITemplateService return Attempt.FailWithStatus(TemplateOperationStatus.MasterTemplateNotFound, template); } + // detect circular references + if (masterTemplateAlias is not null + && masterTemplate is not null + && await HasCircularReference(masterTemplateAlias, template, masterTemplate)) + { + return Attempt.FailWithStatus(TemplateOperationStatus.CircularMasterTemplateReference, template); + } + await SetMasterTemplateAsync(template, masterTemplate, userKey); EventMessages eventMessages = EventMessagesFactory.Get(); @@ -413,4 +421,42 @@ public class TemplateService : RepositoryService, ITemplateService private static bool IsValidAlias(string alias) => alias.IsNullOrWhiteSpace() == false && alias.Length <= 255; + + private async Task HasCircularReference(string parsedMasterTemplateAlias, ITemplate template, ITemplate masterTemplate) + { + // quick check without extra DB calls as we already have both templates + if (parsedMasterTemplateAlias.IsNullOrWhiteSpace() is false + && masterTemplate.MasterTemplateAlias is not null + && masterTemplate.MasterTemplateAlias.Equals(template.Alias)) + { + return true; + } + + var processedTemplates = new List { template, masterTemplate }; + return await HasRecursiveCircularReference(processedTemplates, masterTemplate.MasterTemplateAlias); + } + + private async Task HasRecursiveCircularReference(List referencedTemplates, string? masterTemplateAlias) + { + if (masterTemplateAlias is null) + { + return false; + } + + if (referencedTemplates.Any(template => template.Alias.Equals(masterTemplateAlias))) + { + return true; + } + + ITemplate? masterTemplate = await GetAsync(masterTemplateAlias); + if (masterTemplate is null) + { + // this should not happen unless somebody manipulated the data by hand as this function is only called between persisted items + return false; + } + + referencedTemplates.Add(masterTemplate); + + return await HasRecursiveCircularReference(referencedTemplates, masterTemplate.MasterTemplateAlias); + } }