From de1a607622161c7f773a5ff44a3a9f932bd5226b Mon Sep 17 00:00:00 2001 From: Steve McConnel Date: Thu, 1 Aug 2024 11:14:39 -0600 Subject: [PATCH] Simulate a FileOk handler when choosing an image file This enforces the image file filter even when users type or paste in a file path that defeats the filter. This fix is motivated by https://issues.bloomlibrary.org/youtrack/issue/BL-13552. Using the DialogAdapters interface prevents having an actual FileOk handler. --- CHANGELOG.md | 4 + .../ImageToolbox/ImageToolboxTests.cs | 72 ++++++++++ .../ImageToolbox/AcquireImageControl.cs | 135 +++++++++++++----- 3 files changed, 176 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7efa67758..c714305d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [SIL.Core] Added macOS support for `GlobalMutex` +### Fixed + +- [SIL.Window.Forms] When choosing a file in the ImageToolbox.AcquireImageControl, a FileOk handler is simulated that verifies the selected file passes the given filter. Users can defeat the filter mechanism by pasting or typing the file name. While the returned filename does not pass the filter, the dialog is reopened until the user either chooses a proper filename or cancels the dialog. The native FileOk handler can prevent the dialog from closing: we can't achieve that. (See BL-13552.) + ## [14.1.1] - 2024-05-23 ### Fixed diff --git a/SIL.Windows.Forms.Tests/ImageToolbox/ImageToolboxTests.cs b/SIL.Windows.Forms.Tests/ImageToolbox/ImageToolboxTests.cs index c255053c3..5b0ad7e6e 100644 --- a/SIL.Windows.Forms.Tests/ImageToolbox/ImageToolboxTests.cs +++ b/SIL.Windows.Forms.Tests/ImageToolbox/ImageToolboxTests.cs @@ -112,5 +112,77 @@ public void CheckMonoForSelectLargeIconView() Assert.IsNotNull(viewOnClickMethod, "MWFFileView no longer has an OnClickViewMenuSubItem(object, EventArgs) method"); } } + + [Test] + public void DoubleCheckFileFilterWorks() + { + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(null, "foo.txt")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(null, "foo.doc")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(null, "foo")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(null, "foo.")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(null, "foo.bar")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(null, "foo.bar.baz")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter("", "foo.txt")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter("", "foo.doc")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter("", "foo")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter("", "foo.")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter("", "foo.bar")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter("", "foo.bar.baz")); + + var filterAll = "All files|*.*"; + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterAll, "foo.txt")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterAll, "foo.doc")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterAll, "foo")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterAll, "foo.")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterAll, "foo.bar")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterAll, "foo.bar.baz")); + + var filterTxtOnly = "Text files|*.txt"; + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtOnly, "foo.txt")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtOnly, "foo.doc")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtOnly, "foo")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtOnly, "foo.")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtOnly, "foo.bar")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtOnly, "foo.txt.baz")); + + var filterTxtAndDoc = "Text files|*.txt|Word files|*.doc"; + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDoc, "foo.txt")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDoc, "foo.doc")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDoc, "foo")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDoc, "foo.")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDoc, "foo.bar")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDoc, "foo.txt.baz")); + + var filterTxtAndDocWithAll = "Text files|*.txt|Word files|*.doc|All files|*.*"; + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDocWithAll, "foo.txt")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDocWithAll, "foo.doc")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDocWithAll, "foo")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDocWithAll, "foo.")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDocWithAll, "foo.bar")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDocWithAll, "foo.txt.baz")); + + var filterCodeSourceFiles = "Image files|*.png;*.jpg;*.jpeg;*.tiff"; + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCodeSourceFiles, "foo.png")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCodeSourceFiles, "foo.jpg")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCodeSourceFiles, "foo.jpeg")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCodeSourceFiles, "foo.tiff")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCodeSourceFiles, "foo")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCodeSourceFiles, "foo.")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCodeSourceFiles, "foo.bar")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCodeSourceFiles, "foo.png.baz")); + + var filterCsCppSourceFiles = "Bitmap files|*.bmp;*.png;*.tiff|JPEG files|*.jpg;*.jpeg"; + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.bmp")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.png")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.tiff")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.jpg")); + Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.jpeg")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.h")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.cs")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.bar")); + Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.png.baz")); + } } } diff --git a/SIL.Windows.Forms/ImageToolbox/AcquireImageControl.cs b/SIL.Windows.Forms/ImageToolbox/AcquireImageControl.cs index 3f9d8b4a7..921aa6fdb 100644 --- a/SIL.Windows.Forms/ImageToolbox/AcquireImageControl.cs +++ b/SIL.Windows.Forms/ImageToolbox/AcquireImageControl.cs @@ -81,54 +81,70 @@ private void GetImageFileFromSystem() { try { + // Used to simulate having a FileOk event handler that can cancel the OK button click. + bool invalidFileChosen; SetMode(Modes.SingleImage); - // The primary thing that OpenFileDialogWithViews buys us is the ability to default to large icons. - // OpenFileDialogWithViews still doesn't let us read (and thus remember) the selected view. - using (var dlg = new OpenFileDialogWithViews(OpenFileDialogWithViews.DialogViewTypes.Large_Icons)) + do { - if (string.IsNullOrEmpty(ImageToolboxSettings.Default.LastImageFolder)) + invalidFileChosen = false; + // The primary thing that OpenFileDialogWithViews buys us is the ability to default to large icons. + // OpenFileDialogWithViews still doesn't let us read (and thus remember) the selected view. + using (var dlg = new OpenFileDialogWithViews(OpenFileDialogWithViews.DialogViewTypes.Large_Icons)) { - dlg.InitialDirectory = Environment.GetFolderPath(Environment.SpecialFolder.MyPictures); - } - else - { - dlg.InitialDirectory = ImageToolboxSettings.Default.LastImageFolder; ; - } - - dlg.Title = "Choose Picture File".Localize("ImageToolbox.FileChooserTitle", "Title shown for a file chooser dialog brought up by the ImageToolbox"); + if (string.IsNullOrEmpty(ImageToolboxSettings.Default.LastImageFolder)) + { + dlg.InitialDirectory = Environment.GetFolderPath(Environment.SpecialFolder.MyPictures); + } + else + { + dlg.InitialDirectory = ImageToolboxSettings.Default.LastImageFolder; ; + } - //NB: disallowed gif because of a .net crash: http://jira.palaso.org/issues/browse/BL-85 - dlg.Filter = "picture files".Localize("ImageToolbox.PictureFiles", "Shown in the file-picking dialog to describe what kind of files the dialog is filtering for") + "(*.png;*.tif;*.tiff;*.jpg;*.jpeg;*.bmp)|*.png;*.tif;*.tiff;*.jpg;*.jpeg;*.bmp;"; + dlg.Title = "Choose Picture File".Localize("ImageToolbox.FileChooserTitle", "Title shown for a file chooser dialog brought up by the ImageToolbox"); - if (DialogResult.OK == dlg.ShowDialog(this.ParentForm)) - { - ImageToolboxSettings.Default.LastImageFolder = Path.GetDirectoryName(dlg.FileName); - ImageToolboxSettings.Default.Save(); + //NB: disallowed gif because of a .net crash: http://jira.palaso.org/issues/browse/BL-85 + dlg.Filter = "picture files".Localize("ImageToolbox.PictureFiles", "Shown in the file-picking dialog to describe what kind of files the dialog is filtering for") + "(*.png;*.tif;*.tiff;*.jpg;*.jpeg;*.bmp)|*.png;*.tif;*.tiff;*.jpg;*.jpeg;*.bmp;"; - try - { - _currentImage = PalasoImage.FromFile(dlg.FileName); - } - catch (Exception err) //for example, http://jira.palaso.org/issues/browse/BL-199 + if (DialogResult.OK == dlg.ShowDialog(this.ParentForm)) { - var msg = "Sorry, there was a problem loading that image.".Localize( - "ImageToolbox.ProblemLoadingImage"); - if (ImageLoadingExceptionReporter!= null) + // Simulate having a FileOk event handler that can cancel the OK button click. + // We would prefer to actually use the event handler since that can prevent the + // dialog from closing, but we can't do that because of using DialogAdapters for + // cross-platform compatibility. It's not worth messing with DialogAdapters at + // this point just to get this feature, which might not be cross-platform. + if (!DoubleCheckFileFilter(dlg.Filter, dlg.FileName)) + { + invalidFileChosen = true; + continue; + } + ImageToolboxSettings.Default.LastImageFolder = Path.GetDirectoryName(dlg.FileName); + ImageToolboxSettings.Default.Save(); + + try { - ImageLoadingExceptionReporter(dlg.FileName, err, msg); + _currentImage = PalasoImage.FromFile(dlg.FileName); } - else + catch (Exception err) //for example, http://jira.palaso.org/issues/browse/BL-199 { - ErrorReport.NotifyUserOfProblem(err, msg); + var msg = "Sorry, there was a problem loading that image.".Localize( + "ImageToolbox.ProblemLoadingImage"); + if (ImageLoadingExceptionReporter!= null) + { + ImageLoadingExceptionReporter(dlg.FileName, err, msg); + } + else + { + ErrorReport.NotifyUserOfProblem(err, msg); + } + + return; } - - return; + _pictureBox.Image = _currentImage.Image; + if (ImageChanged != null) + ImageChanged.Invoke(this, null); } - _pictureBox.Image = _currentImage.Image; - if (ImageChanged != null) - ImageChanged.Invoke(this, null); } - } + } while (invalidFileChosen); } finally { @@ -136,6 +152,55 @@ private void GetImageFileFromSystem() } } + /// + /// Return true if the filePath truly passes the filtering of the filterString. + /// People can defeat the filtering by typing or pasting a file path into the file path box. + /// + /// filter string like those used in file dialogs + /// file path returned by a file dialog + internal static bool DoubleCheckFileFilter(string filterString, string filePath) + { + if (string.IsNullOrEmpty(filterString)) + return true; // no filter, so everything passes + if (string.IsNullOrEmpty(filePath)) + return false; // no file, so nothing passes + var filterSections = filterString.Split('|'); + if (filterSections.Length < 2) + return true; // no filter, so everything passes + var fileName = Path.GetFileName(filePath); + for (int i = 1; i < filterSections.Length; i += 2) + { + if (PassesFilter(filterSections[i], fileName)) + return true; + } + return false; + } + private static bool PassesFilter(string filterList, string fileName) + { + var parts = filterList.Split(';'); + foreach (var part in parts) + { + if (part == "*.*" || part == "*") + return true; + var filter = part.Trim(); + if (filter.StartsWith("*")) + { + filter = filter.Substring(1); + if (fileName.EndsWith(filter, StringComparison.InvariantCultureIgnoreCase)) + return true; + } + else if (filter.EndsWith("*")) + { + filter = filter.Substring(0, filter.Length - 1); + if (fileName.StartsWith(filter, StringComparison.InvariantCultureIgnoreCase)) + return true; + } + else if (fileName.Equals(filter, StringComparison.InvariantCultureIgnoreCase)) + return true; + } + return false; + } + private void OpenFileFromDrag(string path) { SetMode(Modes.SingleImage);