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.
sillsdev · Aug 1, 2024 · de1a607 · de1a607
1 parent e5745ec
commit de1a607
Show file tree

Hide file tree

Showing 3 changed files with 176 additions and 35 deletions.
diff --git 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
@@ -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
@@ -81,61 +81,126 @@ 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
 			{
 				_actModal = false;
 			}
 		}
 
+		/// <summary>
+		/// 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.
+		/// </summary>
+		/// <param name="filterString">filter string like those used in file dialogs</param>
+		/// <param name="filePath">file path returned by a file dialog</param>
+		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);