Skip to content

Add validation for Old and New URL in both the interface and CSV import #175

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+31 −1
Open

Add validation for Old and New URL in both the interface and CSV import #175

Show file tree
Hide file tree
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
20 changes: 19 additions & 1 deletion src/Geta.NotFoundHandler.Admin/Areas/GetaNotFoundHandlerAdmin/Pages/Models/RedirectModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,35 @@
using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using Geta.NotFoundHandler.Core.Redirects;

namespace Geta.NotFoundHandler.Admin.Pages.Geta.NotFoundHandler.Admin.Models
{
public class RedirectModel
public class RedirectModel : IValidatableObject
{
public Guid? Id { get; set; }

[Required]
public string OldUrl { get; set; }

[Required]
public string NewUrl { get; set; }

public bool WildCardSkipAppend { get; set; }

public RedirectType RedirectType { get; set; }

public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
{
if (!Uri.IsWellFormedUriString(OldUrl, UriKind.RelativeOrAbsolute))
{
yield return new ValidationResult("Old Url must be a valid absolute or relative url.", new[] { nameof(OldUrl) });
}

if (!Uri.IsWellFormedUriString(NewUrl, UriKind.RelativeOrAbsolute))
{
yield return new ValidationResult("New Url must be a valid absolute or relative url.", new[] { nameof(NewUrl) });
}
}
}
}
12 changes: 12 additions & 0 deletions src/Geta.NotFoundHandler/Core/Redirects/RedirectsCsvParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ private CustomRedirectCollection Load(IEnumerable<CsvImportModel> csvImportModel

foreach (var item in csvImportModels)
{
if (!IsValidRedirect(item))
{
_logger.LogWarning("NotFoundHandler: Skipping import of invalid redirect. OldUrl: {oldUrl}; NewUrl: {newUrl}", item.OldUrl, item.NewUrl);
continue;
}

// Create new custom redirect nodes
var redirectType = GetRedirectType(item.RedirectType);
var skipWildCardAppend = GetSkipWildCardAppend(item.WildcardSkippedAppend);
Expand All @@ -56,6 +62,12 @@ private CustomRedirectCollection Load(IEnumerable<CsvImportModel> csvImportModel
return redirects;
}

private static bool IsValidRedirect(CsvImportModel model)
{
return Uri.IsWellFormedUriString(model.OldUrl, UriKind.RelativeOrAbsolute)
&& Uri.IsWellFormedUriString(model.NewUrl, UriKind.RelativeOrAbsolute);
}
Comment on lines +65 to +69
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The URL validation logic Uri.IsWellFormedUriString(url, UriKind.RelativeOrAbsolute) is also used in RedirectModel.Validate. While not a major issue with only two occurrences, consider centralizing this logic into a shared static utility method if this validation pattern is expected to be used elsewhere or might evolve. This would improve maintainability by ensuring a single source of truth for what constitutes a valid redirect URL string.

For example, you could have a utility class:

public static class UrlValidationHelper
{
    public static bool IsValidRedirectUri(string uriString)
    {
        return Uri.IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute);
    }
}

Then, both RedirectModel.Validate and this IsValidRedirect method could call UrlValidationHelper.IsValidRedirectUri().


private static bool GetSkipWildCardAppend(string skipWildCardAttr)
{
if (!string.IsNullOrWhiteSpace(skipWildCardAttr) && bool.TryParse(skipWildCardAttr, out var skipWildCardAppend))
Expand Down