From 3604a52d74fcc67955fb123ecd1b6b2dc265107e Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Wed, 21 Apr 2010 10:01:57 -0700 Subject: [PATCH] Corrected slugs validation and added unit tests --- .../Common/Services/RoutableServiceTests.cs | 18 +++++++ .../Core/Common/Services/RoutableService.cs | 3 +- .../Orchard.Blogs/Drivers/BlogPostDriver.cs | 49 +------------------ 3 files changed, 21 insertions(+), 49 deletions(-) diff --git a/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs b/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs index b587f0c54..1a3b18f47 100644 --- a/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs +++ b/src/Orchard.Core.Tests/Common/Services/RoutableServiceTests.cs @@ -52,6 +52,24 @@ namespace Orchard.Core.Tests.Common.Services { Assert.That(thing.Slug, Is.EqualTo("please-do-not-use-any-of-the-following-characters-in-your-slugs-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"-\"")); } + [Test] + public void EmptySlugsShouldBeConsideredValid() { + // so that automatic generation on Publish occurs + Assert.That(_routableService.IsSlugValid(null), Is.True); + Assert.That(_routableService.IsSlugValid(String.Empty), Is.True); + Assert.That(_routableService.IsSlugValid(" "), Is.True); + } + + [Test] + public void InvalidCharacterShouldBeRefusedInSlugs() { + Assert.That(_routableService.IsSlugValid("aaaa-_aaaa"), Is.True); + + foreach (var c in @"/:?#[]@!$&'()*+,;= ") { + Assert.That(_routableService.IsSlugValid("a" + c + "b"), Is.False); + } + } + + [Test] public void VeryLongStringTruncatedTo1000Chars() { var contentManager = _container.Resolve(); diff --git a/src/Orchard.Web/Core/Common/Services/RoutableService.cs b/src/Orchard.Web/Core/Common/Services/RoutableService.cs index 5b901ae14..cfddb58b5 100644 --- a/src/Orchard.Web/Core/Common/Services/RoutableService.cs +++ b/src/Orchard.Web/Core/Common/Services/RoutableService.cs @@ -74,7 +74,8 @@ namespace Orchard.Core.Common.Services { } public bool IsSlugValid(string slug) { - return slug == null || String.IsNullOrEmpty(slug.Trim()) || !Regex.IsMatch(slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$"); + // see http://tools.ietf.org/html/rfc3987 for prohibited chars + return slug == null || String.IsNullOrEmpty(slug.Trim()) || Regex.IsMatch(slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$"); } public bool ProcessSlug(RoutableAspect part) diff --git a/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogPostDriver.cs b/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogPostDriver.cs index adec3f1ff..8ac6ab501 100644 --- a/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogPostDriver.cs +++ b/src/Orchard.Web/Modules/Orchard.Blogs/Drivers/BlogPostDriver.cs @@ -1,19 +1,15 @@ using System; -using System.Linq; -using System.Text.RegularExpressions; using System.Web.Routing; using JetBrains.Annotations; using Orchard.Blogs.Models; using Orchard.Blogs.Services; using Orchard.ContentManagement; using Orchard.ContentManagement.Drivers; -using Orchard.Core.Common.Models; using Orchard.Core.Common.Services; using Orchard.Localization; -using Orchard.UI.Notify; namespace Orchard.Blogs.Drivers { - [UsedImplicitly] + [UsedImplicitly] public class BlogPostDriver : ContentItemDriver { public IOrchardServices Services { get; set; } private readonly IBlogPostService _blogPostService; @@ -78,54 +74,11 @@ namespace Orchard.Blogs.Drivers { protected override DriverResult Editor(BlogPost post, IUpdateModel updater) { updater.TryUpdateModel(post, Prefix, null, null); - //todo: (heskew) something better needs to be done with this...still feels shoehorned in here - ProcessSlug(post, updater); - DateTime scheduled; if (DateTime.TryParse(string.Format("{0} {1}", post.ScheduledPublishUtcDate, post.ScheduledPublishUtcTime), out scheduled)) post.ScheduledPublishUtc = scheduled; return Editor(post); } - - private void ProcessSlug(BlogPost post, IUpdateModel updater) { - _routableService.FillSlug(post.As()); - - if (string.IsNullOrEmpty(post.Slug)) { - return; - - // OR - - //updater.AddModelError("Routable.Slug", T("The slug is required.").ToString()); - //return; - } - - if (!Regex.IsMatch(post.Slug, @"^[^/:?#\[\]@!$&'()*+,;=\s]+$")) { - //todo: (heskew) get rid of the hard-coded prefix - updater.AddModelError("Routable.Slug", T("Please do not use any of the following characters in your slugs: \"/\", \":\", \"?\", \"#\", \"[\", \"]\", \"@\", \"!\", \"$\", \"&\", \"'\", \"(\", \")\", \"*\", \"+\", \",\", \";\", \"=\". No spaces are allowed (please use dashes or underscores instead).").ToString()); - return; - } - - var slugsLikeThis = _blogPostService.Get(post.Blog, VersionOptions.Published).Where( - p => p.Slug.StartsWith(post.Slug, StringComparison.OrdinalIgnoreCase) && - p.Id != post.Id).Select(p => p.Slug); - - //todo: (heskew) need better messages - if (slugsLikeThis.Count() > 0) { - //todo: (heskew) need better messages - Services.Notifier.Warning(T("A different blog post is already published with this same slug.")); - - if (post.ContentItem.VersionRecord == null || post.ContentItem.VersionRecord.Published) - { - var originalSlug = post.Slug; - //todo: (heskew) make auto-uniqueness optional - post.Slug = _routableService.GenerateUniqueSlug(post.Slug, slugsLikeThis); - - if (originalSlug != post.Slug) - Services.Notifier.Warning(T("Slugs in conflict. \"{0}\" is already set for a previously created blog post so this post now has the slug \"{1}\"", - originalSlug, post.Slug)); - } - } - } } } \ No newline at end of file