From 4465d50a317694d7224ccafb454701f2629294f8 Mon Sep 17 00:00:00 2001 From: "kay.one" Date: Sat, 31 Aug 2013 13:31:58 -0700 Subject: [PATCH] added path validation to add series/ recent folders. --- NzbDrone.Api/NzbDrone.Api.csproj | 1 + NzbDrone.Api/RootFolders/RootFolderModule.cs | 3 + NzbDrone.Api/Series/SeriesModule.cs | 6 +- NzbDrone.Api/Validation/PathValidator.cs | 18 ++++++ .../Validation/RuleBuilderExtensions.cs | 9 ++- .../EnsureThat/EnsureStringExtensions.cs | 30 ++-------- NzbDrone.Common/PathExtensions.cs | 27 +++++++++ .../Validation/RuleBuilderExtensions.cs | 1 + NzbDrone.Integration.Test/IntegrationTest.cs | 7 ++- .../RootFolderIntegrationTest.cs | 12 ++++ .../SeriesIntegrationTest.cs | 6 -- UI/AddSeries/RootFolders/Collection.js | 2 +- UI/AddSeries/RootFolders/Layout.js | 11 +++- UI/AddSeries/RootFolders/LayoutTemplate.html | 9 ++- UI/AddSeries/RootFolders/Model.js | 1 + UI/AddSeries/SearchResultView.js | 3 +- UI/Content/theme.less | 11 ++-- UI/Mixins/AsValidatedView.js | 60 +++++++++---------- UI/jQuery/jquery.validation.js | 7 ++- 19 files changed, 141 insertions(+), 83 deletions(-) create mode 100644 NzbDrone.Api/Validation/PathValidator.cs diff --git a/NzbDrone.Api/NzbDrone.Api.csproj b/NzbDrone.Api/NzbDrone.Api.csproj index 5210ef0a6..d0a03bd17 100644 --- a/NzbDrone.Api/NzbDrone.Api.csproj +++ b/NzbDrone.Api/NzbDrone.Api.csproj @@ -164,6 +164,7 @@ + diff --git a/NzbDrone.Api/RootFolders/RootFolderModule.cs b/NzbDrone.Api/RootFolders/RootFolderModule.cs index 5c13a736c..7bcc4aa52 100644 --- a/NzbDrone.Api/RootFolders/RootFolderModule.cs +++ b/NzbDrone.Api/RootFolders/RootFolderModule.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using NzbDrone.Core.RootFolders; using NzbDrone.Api.Mapping; +using NzbDrone.Api.Validation; namespace NzbDrone.Api.RootFolders { @@ -17,6 +18,8 @@ namespace NzbDrone.Api.RootFolders GetResourceById = GetRootFolder; CreateResource = CreateRootFolder; DeleteResource = DeleteFolder; + + SharedValidator.RuleFor(c=>c.Path).IsValidPath(); } private RootFolderResource GetRootFolder(int id) diff --git a/NzbDrone.Api/Series/SeriesModule.cs b/NzbDrone.Api/Series/SeriesModule.cs index 57caeb7a6..fe7b820a6 100644 --- a/NzbDrone.Api/Series/SeriesModule.cs +++ b/NzbDrone.Api/Series/SeriesModule.cs @@ -31,10 +31,10 @@ namespace NzbDrone.Api.Series SharedValidator.RuleFor(s => s.QualityProfileId).ValidId(); - PutValidator.RuleFor(s => s.Path).NotEmpty(); + PutValidator.RuleFor(s => s.Path).IsValidPath(); - PostValidator.RuleFor(s => s.Path).NotEmpty().When(s => String.IsNullOrEmpty(s.RootFolderPath)); - PostValidator.RuleFor(s => s.RootFolderPath).NotEmpty().When(s => String.IsNullOrEmpty(s.Path)); + PostValidator.RuleFor(s => s.Path).IsValidPath().When(s => String.IsNullOrEmpty(s.RootFolderPath)); + PostValidator.RuleFor(s => s.RootFolderPath).IsValidPath().When(s => String.IsNullOrEmpty(s.Path)); PostValidator.RuleFor(s => s.Title).NotEmpty(); } diff --git a/NzbDrone.Api/Validation/PathValidator.cs b/NzbDrone.Api/Validation/PathValidator.cs new file mode 100644 index 000000000..bc055a98c --- /dev/null +++ b/NzbDrone.Api/Validation/PathValidator.cs @@ -0,0 +1,18 @@ +using FluentValidation.Validators; +using NzbDrone.Common; + +namespace NzbDrone.Api.Validation +{ + public class PathValidator : PropertyValidator + { + public PathValidator() + : base("Invalid Path") + { + } + + protected override bool IsValid(PropertyValidatorContext context) + { + return context.PropertyValue.ToString().IsPathValid(); + } + } +} \ No newline at end of file diff --git a/NzbDrone.Api/Validation/RuleBuilderExtensions.cs b/NzbDrone.Api/Validation/RuleBuilderExtensions.cs index 23dc34c77..b142f5a56 100644 --- a/NzbDrone.Api/Validation/RuleBuilderExtensions.cs +++ b/NzbDrone.Api/Validation/RuleBuilderExtensions.cs @@ -1,4 +1,6 @@ -using System.Text.RegularExpressions; +using System; +using System.Linq.Expressions; +using System.Text.RegularExpressions; using FluentValidation; using FluentValidation.Validators; @@ -20,5 +22,10 @@ namespace NzbDrone.Api.Validation { return ruleBuilder.SetValidator(new RegularExpressionValidator("^http(s)?://", RegexOptions.IgnoreCase)).WithMessage("must start with http:// or https://"); } + + public static IRuleBuilderOptions IsValidPath(this IRuleBuilder ruleBuilder) + { + return ruleBuilder.SetValidator(new PathValidator()); + } } } \ No newline at end of file diff --git a/NzbDrone.Common/EnsureThat/EnsureStringExtensions.cs b/NzbDrone.Common/EnsureThat/EnsureStringExtensions.cs index 9a0275894..0cd595464 100644 --- a/NzbDrone.Common/EnsureThat/EnsureStringExtensions.cs +++ b/NzbDrone.Common/EnsureThat/EnsureStringExtensions.cs @@ -95,8 +95,7 @@ namespace NzbDrone.Common.EnsureThat return param; } - private static readonly Regex windowsInvalidPathRegex = new Regex(@"[/*<>""|]", RegexOptions.Compiled); - private static readonly Regex windowsPathRegex = new Regex(@"^[a-zA-Z]:\\", RegexOptions.Compiled); + [DebuggerStepThrough] public static Param IsValidPath(this Param param) @@ -104,31 +103,14 @@ namespace NzbDrone.Common.EnsureThat if (string.IsNullOrWhiteSpace(param.Value)) throw ExceptionFactory.CreateForParamValidation(param.Name, ExceptionMessages.EnsureExtensions_IsNotNullOrWhiteSpace); + if (param.Value.IsPathValid()) return param; + if (OsInfo.IsLinux) { - if (!param.Value.StartsWith(Path.DirectorySeparatorChar.ToString())) - { - throw ExceptionFactory.CreateForParamValidation(param.Name, string.Format("value [{0}] is not a valid *nix path. paths must start with /", param.Value)); - } - } - else - { - if (windowsInvalidPathRegex.IsMatch(param.Value)) - { - throw ExceptionFactory.CreateForParamValidation(param.Name, string.Format("value [{0}] is not a valid Windows path. It contains invalid characters", param.Value)); - } - - //Network path - if (param.Value.StartsWith(Path.DirectorySeparatorChar.ToString())) return param; - - if (!windowsPathRegex.IsMatch(param.Value)) - { - throw ExceptionFactory.CreateForParamValidation(param.Name, string.Format("value [{0}] is not a valid Windows path. paths must be a full path eg. C:\\Windows", param.Value)); - } + throw ExceptionFactory.CreateForParamValidation(param.Name, string.Format("value [{0}] is not a valid *nix path. paths must start with /", param.Value)); } - - - return param; + + throw ExceptionFactory.CreateForParamValidation(param.Name, string.Format("value [{0}] is not a valid Windows path. paths must be a full path eg. C:\\Windows", param.Value)); } } } diff --git a/NzbDrone.Common/PathExtensions.cs b/NzbDrone.Common/PathExtensions.cs index 374d2372a..2cdd5dd44 100644 --- a/NzbDrone.Common/PathExtensions.cs +++ b/NzbDrone.Common/PathExtensions.cs @@ -1,5 +1,6 @@ using System; using System.IO; +using System.Text.RegularExpressions; using NzbDrone.Common.EnsureThat; using NzbDrone.Common.EnvironmentInfo; @@ -45,6 +46,32 @@ namespace NzbDrone.Common return String.Equals(firstPath.CleanFilePath(), secondPath.CleanFilePath(), StringComparison.InvariantCultureIgnoreCase); } + private static readonly Regex WindowsInvalidPathRegex = new Regex(@"[/*<>""|]", RegexOptions.Compiled); + private static readonly Regex WindowsPathRegex = new Regex(@"^[a-zA-Z]:\\", RegexOptions.Compiled); + + public static bool IsPathValid(this string path) + { + if (OsInfo.IsLinux && !path.StartsWith(Path.DirectorySeparatorChar.ToString())) + { + return false; + } + if (WindowsInvalidPathRegex.IsMatch(path)) + { + return false; + } + + //Network path + if (path.StartsWith(Path.DirectorySeparatorChar.ToString())) return true; + + if (!WindowsPathRegex.IsMatch(path)) + { + return false; + } + + return true; + } + + public static bool ContainsInvalidPathChars(this string text) { return text.IndexOfAny(Path.GetInvalidPathChars()) >= 0; diff --git a/NzbDrone.Core/Validation/RuleBuilderExtensions.cs b/NzbDrone.Core/Validation/RuleBuilderExtensions.cs index 6401fb475..da8609e84 100644 --- a/NzbDrone.Core/Validation/RuleBuilderExtensions.cs +++ b/NzbDrone.Core/Validation/RuleBuilderExtensions.cs @@ -23,6 +23,7 @@ namespace NzbDrone.Core.Validation public static IRuleBuilderOptions ValidRootUrl(this IRuleBuilder ruleBuilder) { + ruleBuilder.SetValidator(new NotEmptyValidator(null)); return ruleBuilder.SetValidator(new RegularExpressionValidator("^http(?:s)?://[a-z0-9-.]+", RegexOptions.IgnoreCase)).WithMessage("must be valid URL that"); } } diff --git a/NzbDrone.Integration.Test/IntegrationTest.cs b/NzbDrone.Integration.Test/IntegrationTest.cs index 3ef126a98..c0bb4b5fd 100644 --- a/NzbDrone.Integration.Test/IntegrationTest.cs +++ b/NzbDrone.Integration.Test/IntegrationTest.cs @@ -1,4 +1,5 @@ -using NLog; +using System.Runtime.CompilerServices; +using NLog; using NLog.Config; using NLog.Targets; using NUnit.Framework; @@ -39,7 +40,7 @@ namespace NzbDrone.Integration.Test LogManager.Configuration.LoggingRules.Add(new LoggingRule("*", LogLevel.Trace, consoleTarget)); } - [SetUp] + [TestFixtureSetUp] public void SmokeTestSetup() { _runner = new NzbDroneRunner(); @@ -63,7 +64,7 @@ namespace NzbDrone.Integration.Test NamingConfig = new ClientBase(RestClient, "config/naming"); } - [TearDown] + [TestFixtureTearDown] public void SmokeTestTearDown() { _runner.KillAll(); diff --git a/NzbDrone.Integration.Test/RootFolderIntegrationTest.cs b/NzbDrone.Integration.Test/RootFolderIntegrationTest.cs index cd1b29cf0..8a888fffa 100644 --- a/NzbDrone.Integration.Test/RootFolderIntegrationTest.cs +++ b/NzbDrone.Integration.Test/RootFolderIntegrationTest.cs @@ -58,5 +58,17 @@ namespace NzbDrone.Integration.Test RootFolders.All().Should().BeEmpty(); } + + [Test] + public void invalid_path_should_return_bad_request() + { + var rootFolder = new RootFolderResource + { + Path = "invalid_path" + }; + + var postResponse = RootFolders.InvalidPost(rootFolder); + postResponse.Should().NotBeEmpty(); + } } } \ No newline at end of file diff --git a/NzbDrone.Integration.Test/SeriesIntegrationTest.cs b/NzbDrone.Integration.Test/SeriesIntegrationTest.cs index 85ef89050..c03940490 100644 --- a/NzbDrone.Integration.Test/SeriesIntegrationTest.cs +++ b/NzbDrone.Integration.Test/SeriesIntegrationTest.cs @@ -9,12 +9,6 @@ namespace NzbDrone.Integration.Test [TestFixture] public class SeriesIntegrationTest : IntegrationTest { - [Test] - public void should_have_no_series_on_start_application() - { - Series.All().Should().BeEmpty(); - } - [Test] public void series_lookup_on_trakt() { diff --git a/UI/AddSeries/RootFolders/Collection.js b/UI/AddSeries/RootFolders/Collection.js index 8f20ac92a..74b9b4555 100644 --- a/UI/AddSeries/RootFolders/Collection.js +++ b/UI/AddSeries/RootFolders/Collection.js @@ -1,4 +1,4 @@ -'use strict'; +'use strict'; define( [ 'backbone', diff --git a/UI/AddSeries/RootFolders/Layout.js b/UI/AddSeries/RootFolders/Layout.js index 89388b2c2..3a75c0f9f 100644 --- a/UI/AddSeries/RootFolders/Layout.js +++ b/UI/AddSeries/RootFolders/Layout.js @@ -7,10 +7,11 @@ define( 'AddSeries/RootFolders/Collection', 'AddSeries/RootFolders/Model', 'Shared/LoadingView', + 'Mixins/AsValidatedView', 'Mixins/AutoComplete' - ], function (Marionette, RootFolderCollectionView, RootFolderCollection, RootFolderModel, LoadingView) { + ], function (Marionette, RootFolderCollectionView, RootFolderCollection, RootFolderModel, LoadingView, AsValidatedView) { - return Marionette.Layout.extend({ + var layout = Marionette.Layout.extend({ template: 'AddSeries/RootFolders/LayoutTemplate', ui: { @@ -55,12 +56,16 @@ define( Path: this.ui.pathInput.val() }); - RootFolderCollection.add(newDir); + this.bindToModelValidation(newDir); newDir.save().done(function () { + RootFolderCollection.add(newDir); self.trigger('folderSelected', {model: newDir}); }); } }); + + return AsValidatedView.apply(layout); + }); diff --git a/UI/AddSeries/RootFolders/LayoutTemplate.html b/UI/AddSeries/RootFolders/LayoutTemplate.html index 2a5d5f350..a62ef128d 100644 --- a/UI/AddSeries/RootFolders/LayoutTemplate.html +++ b/UI/AddSeries/RootFolders/LayoutTemplate.html @@ -3,10 +3,13 @@

Select Folder

-
+
+
  - - + +
{{#if items}}

Recent Folders

diff --git a/UI/AddSeries/RootFolders/Model.js b/UI/AddSeries/RootFolders/Model.js index 13e20a7f0..2c54f6ae3 100644 --- a/UI/AddSeries/RootFolders/Model.js +++ b/UI/AddSeries/RootFolders/Model.js @@ -4,6 +4,7 @@ define( 'backbone' ], function (Backbone) { return Backbone.Model.extend({ + urlRoot : window.ApiRoot + '/rootfolder', defaults: { freeSpace: 0 } diff --git a/UI/AddSeries/SearchResultView.js b/UI/AddSeries/SearchResultView.js index 5fb6101f0..e7d16631a 100644 --- a/UI/AddSeries/SearchResultView.js +++ b/UI/AddSeries/SearchResultView.js @@ -38,7 +38,7 @@ define( this.listenTo(App.vent, Config.Events.ConfigUpdatedEvent, this._onConfigUpdated); this.listenTo(this.model, 'change', this.render); - this.listenTo(RootFolders, 'change', this.render); + this.listenTo(RootFolders, 'all', this.render); this.rootFolderLayout = new RootFolderLayout(); this.listenTo(this.rootFolderLayout, 'folderSelected', this._setRootFolder); @@ -108,6 +108,7 @@ define( _setRootFolder: function (options) { App.vent.trigger(App.Commands.CloseModalCommand); this.ui.rootFolder.val(options.model.id); + this._rootFolderChanged(); }, _addSeries: function () { diff --git a/UI/Content/theme.less b/UI/Content/theme.less index 28fc141ad..4c8c3a1e8 100644 --- a/UI/Content/theme.less +++ b/UI/Content/theme.less @@ -39,8 +39,6 @@ } } - - .page-toolbar { margin-top : 10px; margin-bottom : 30px; @@ -78,8 +76,6 @@ th { } } - - a, .btn { i { cursor : pointer; @@ -91,7 +87,6 @@ a, .btn { background-color : white; } - body { background-color : #1c1c1c; background-image : url('../Content/Images/pattern.png'); @@ -146,3 +141,9 @@ footer { background-color : transparent; box-shadow : none; } + +.validation-errors { + i { + padding-right : 5px; + } +} diff --git a/UI/Mixins/AsValidatedView.js b/UI/Mixins/AsValidatedView.js index a8c6cf06e..d074b054e 100644 --- a/UI/Mixins/AsValidatedView.js +++ b/UI/Mixins/AsValidatedView.js @@ -12,36 +12,48 @@ define( var originalOnClose = this.prototype.onClose; var originalBeforeClose = this.prototype.onBeforeClose; + var errorHandler = function (response) { - this.prototype.onRender = function () { - - Validation.bind(this); + if (response.status === 400) { + var view = this; + var validationErrors = JSON.parse(response.responseText); + _.each(validationErrors, function (error) { + view.$el.processServerError(error); + }); + } + }; - if (!this.originalSync && this.model) { - var self = this; - this.originalSync = this.model.sync; + var validatedSync = function (method, model,options) { + this.$el.removeAllErrors(); + arguments[2].isValidatedCall = true; + return model._originalSync.apply(this, arguments).fail(errorHandler.bind(this)); + }; + var bindToModel = function (model) { - var boundHandler = errorHandler.bind(this); + if (!model._originalSync) { + model._originalSync = model.sync; + model.sync = validatedSync.bind(this); + } + }; - this.model.sync = function () { - self.$el.removeAllErrors(); + this.prototype.onRender = function () { - arguments[2].isValidatedCall = true; + Validation.bind(this); + this.bindToModelValidation = bindToModel.bind(this); - return self.originalSync.apply(this, arguments).fail(boundHandler); - }; + if (this.model) { + this.bindToModelValidation(this.model); } - if (this.model) { - if (originalOnRender) { - originalOnRender.call(this); - } + if (originalOnRender) { + originalOnRender.call(this); } }; + this.prototype.onBeforeClose = function () { if (this.model) { @@ -65,22 +77,6 @@ define( }; - var errorHandler = function (response) { - - if (response.status === 400) { - - var view = this; - - var validationErrors = JSON.parse(response.responseText); - - _.each(validationErrors, function (error) { - view.$el.processServerError(error); - }); - } - }; - - - return this; }; }); diff --git a/UI/jQuery/jquery.validation.js b/UI/jQuery/jquery.validation.js index fda5087c5..ca03d0aa7 100644 --- a/UI/jQuery/jquery.validation.js +++ b/UI/jQuery/jquery.validation.js @@ -8,6 +8,10 @@ define( var validationName = error.propertyName.toLowerCase(); + this.find('.validation-errors') + .addClass('alert alert-error') + .append('
' + error.errorMessage + '
'); + var input = this.find('[name]').filter(function () { return this.name.toLowerCase() === validationName; }); @@ -40,11 +44,12 @@ define( }; $.fn.addFormError = function (error) { - this.find('.control-group').parent().prepend('
'+ error.errorMessage +'
') + this.find('.control-group').parent().prepend('
' + error.errorMessage + '
') }; $.fn.removeAllErrors = function () { this.find('.error').removeClass('error'); + this.find('.validation-errors').removeClass('alert').removeClass('alert-error').html(''); this.find('.validation-error').remove(); return this.find('.help-inline.error-message').remove(); };