Skip to content

Commit

Permalink
Do not allow save of invalid domains
Browse files Browse the repository at this point in the history
  • Loading branch information
bergmania committed Aug 7, 2024
1 parent 0f16dae commit 7d4cb08
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ public async Task<IActionResult> Update(
.WithDetail("One or more of the specified domain names were conflicting with domain assignments to other content items.")
.WithExtension("conflictingDomainNames", _domainPresentationFactory.CreateDomainAssignmentModels(result.Result.ConflictingDomains.EmptyNull()))
.Build()),
DomainOperationStatus.InvalidDomainName => BadRequest(problemDetailsBuilder
.WithTitle("Invalid domain name detected")
.WithDetail("One or more of the specified domain names were invalid.")
.Build()),
_ => StatusCode(StatusCodes.Status500InternalServerError, problemDetailsBuilder
.WithTitle("Unknown domain update operation status.")
.Build()),
Expand Down
6 changes: 6 additions & 0 deletions src/Umbraco.Core/Services/DomainService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Umbraco.Cms.Core.Models.ContentEditing;
using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Core.Persistence.Repositories;
using Umbraco.Cms.Core.Routing;
using Umbraco.Cms.Core.Scoping;
using Umbraco.Cms.Core.Services.OperationStatus;
using Umbraco.Extensions;
Expand Down Expand Up @@ -201,6 +202,11 @@ public async Task<Attempt<DomainUpdateResult, DomainOperationStatus>> UpdateDoma
foreach (DomainModel domainModel in updateModel.Domains)
{
domainModel.DomainName = domainModel.DomainName.ToLowerInvariant();

if(Uri.IsWellFormedUriString(domainModel.DomainName, UriKind.RelativeOrAbsolute) is false)
{
return Attempt.FailWithStatus(DomainOperationStatus.InvalidDomainName, new DomainUpdateResult());
}

Check warning on line 209 in src/Umbraco.Core/Services/DomainService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (release/14.2)

❌ New issue: Complex Method

UpdateDomainsAsync has a cyclomatic complexity of 9, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
}

// make sure we're not attempting to assign duplicate domains
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ public enum DomainOperationStatus
ContentNotFound,
LanguageNotFound,
DuplicateDomainName,
ConflictingDomainName
ConflictingDomainName,
InvalidDomainName
}
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,22 @@ public async Task Cannot_Assign_Duplicate_Domains(string domainName)
Assert.AreEqual(DomainOperationStatus.DuplicateDomainName, result.Status);
}

[TestCase("https://*.umbraco.com")]
[TestCase("&#€%#€")]
[TestCase("¢”$¢”¢$≈{")]
public async Task Cannot_Assign_Invalid_Domains(string domainName)
{
var domainService = GetRequiredService<IDomainService>();
var updateModel = new DomainsUpdateModel
{
Domains = new DomainModel { DomainName = domainName, IsoCode = Cultures.First() }.Yield()
};

var result = await domainService.UpdateDomainsAsync(Root.Key, updateModel);
Assert.IsFalse(result.Success);
Assert.AreEqual(DomainOperationStatus.InvalidDomainName, result.Status);
}

[Test]
public async Task Cannot_Assign_Already_Used_Domains()
{
Expand Down

0 comments on commit 7d4cb08

Please sign in to comment.