Add safe methods to work with huge files

FEAT: The Npp.TryGetLengthAsInt method is preferred over IScintillaGateway.GetLength, and the Npp.TryGetText method is preferred over IScintillaGateway.GetText, because those methods avoid the risk of crashing Notepad++ when attempting to read a file too large for a C# string FIX: avoid crash on too-large integers in selection-remembering form FEAT: add propagation of global constants to keys (not just values) in the translation file.
molsonkiko · Aug 1, 2024 · 8441865 · 8441865
1 parent f96d23f
commit 8441865
Show file tree

Hide file tree

Showing 12 changed files with 142 additions and 36 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,7 +19,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### To Be Fixed
 
 - Closing HTML/XML tags works inconsistently in Notepad++ v7.3.3.
-- Avoid plugin crash when too-large int values are entered in the selection-remembering form.
 - Holding down `Enter` in a multiline textbox does not add multiple new lines; it only adds one newline on keyup.
 
 ## [0.0.4] - (UNRELEASED) YYYY-MM-DD
@@ -29,6 +28,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 1. Make it much easier to [include third-party dependencies](/docs/README.md#loading-third-party-dependencies) in your plugin.
 2. Added the ability to [translate the plugin into other languages](/README.md#translating-your-plugin-to-another-language).
 3. Made it so all forms subclass a base class, making it easier to implement recommended methods.
+4. `Npp.TryGetLengthAsInt` and `Npp.TryGetText` methods, which gracefully handle files that are too large to put all their text in a string.
 
 ### Fixed
 
@@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 4. Fix bug where, if a setting in the config file had an invalid value (for example, a numeric setting having a value of `blah`), there might be an uncaught exception that would cause Notepad++ to crash. This bug appears to be most likely to occur when the localization is *not* set to `en-us`.
 5. Fix bug (related to fixed bug 4 above) where *all settings with non-integer floating-point values* would cause Notepad++ to crash on start-up if the localization used `,` as the decimal separator. 
 6. Fix bug with [makerelease.bat](/makerelease.bat) where it did not properly copy the dependency DLLs into the zip files.
+7. Fix bug when too-large integers are entered in the selection-remembering form.
 
 ## [0.0.3] - 2024-02-26
 

diff --git a/NppCSharpPluginPack/PluginInfrastructure/IScintillaGateway.cs b/NppCSharpPluginPack/PluginInfrastructure/IScintillaGateway.cs
@@ -55,7 +55,15 @@ public interface IScintillaGateway
         void ClearDocumentStyle();
 
         /// <summary>Returns the number of bytes in the document. (Scintilla feature 2006)</summary>
-        int GetLength();
+        long GetLength();
+
+        /// <summary>
+        /// If <see cref="GetLength"/> would return a value greater than <see cref="int.MaxValue"/>, return false and set result to -1.<br></br>
+        /// Otherwise, return true and set result to the number of bytes in the document.
+        /// </summary>
+        /// <param name="result"></param>
+        /// <returns></returns>
+        bool TryGetLengthAsInt(out int result);
 
         /// <summary>Returns the character byte at the position. (Scintilla feature 2007)</summary>
         int GetCharAt(int pos);
@@ -909,9 +917,9 @@ public interface IScintillaGateway
         unsafe void SetText(string text);
 
         /// <summary>
-        /// Retrieve all the text in the document (or the first length chars of the document).
-        /// Returns number of characters retrieved.
-        /// Result is NUL-terminated.
+        /// You may wish to use Npp.TryGetLengthAsInt instead, as it indicates when the length of the document is too great.<br></br>
+        /// If length = -1 and the document has more than <see cref="int.MaxValue"/> characters, return "".<br></br>
+        /// Otherwise, returns all the text in the document if length = -1 (or the first length chars of the document).<br></br>
         /// (Scintilla feature 2182)
         /// </summary>
         unsafe string GetText(int length = -1);

diff --git a/NppCSharpPluginPack/PluginInfrastructure/ScintillaGateway.cs b/NppCSharpPluginPack/PluginInfrastructure/ScintillaGateway.cs
@@ -175,9 +175,21 @@ public void ClearDocumentStyle()
         }
 
         /// <summary>Returns the number of bytes in the document. (Scintilla feature 2006)</summary>
-        public int GetLength()
+        public long GetLength()
         {
-            return (int)Win32.SendMessage(scintilla, SciMsg.SCI_GETLENGTH, (IntPtr) Unused, (IntPtr) Unused);
+            return (long)Win32.SendMessage(scintilla, SciMsg.SCI_GETLENGTH, (IntPtr) Unused, (IntPtr) Unused);
+        }
+
+        public bool TryGetLengthAsInt(out int result)
+        {
+            long longRes = GetLength();
+            if (longRes > int.MaxValue)
+            {
+                result = -1;
+                return false;
+            }
+            result = (int)longRes;
+            return true;
         }
 
         /// <summary>Returns the character byte at the position. (Scintilla feature 2007)</summary>
@@ -1769,16 +1781,10 @@ public unsafe void SetText(string text)
             }
         }
 
-        /// <summary>
-        /// Retrieve all the text in the document.
-        /// Returns number of characters retrieved.
-        /// Result is NUL-terminated.
-        /// (Scintilla feature 2182)
-        /// </summary>
-        public unsafe string GetText(int length=-1)
+        public unsafe string GetText(int length = -1)
         {
-            if (length < 1)
-                length = Win32.SendMessage(scintilla, SciMsg.SCI_GETTEXT, (IntPtr)length, (IntPtr)Unused).ToInt32();
+            if (length < 1 && !TryGetLengthAsInt(out length))
+                return "";
             byte[] textBuffer = new byte[length];
             fixed (byte* textPtr = textBuffer)
             {

diff --git a/NppCSharpPluginPack/Properties/AssemblyInfo.cs b/NppCSharpPluginPack/Properties/AssemblyInfo.cs
@@ -28,5 +28,5 @@
 //      Build Number
 //      Revision
 //
-[assembly: AssemblyVersion("0.0.3.9")]
-[assembly: AssemblyFileVersion("0.0.3.9")]
+[assembly: AssemblyVersion("0.0.3.10")]
+[assembly: AssemblyFileVersion("0.0.3.10")]
diff --git a/NppCSharpPluginPack/Tests/UserInterfaceTests.cs b/NppCSharpPluginPack/Tests/UserInterfaceTests.cs
@@ -131,7 +131,12 @@ public static bool ExecuteFileManipulation(FileManipulation command, List<string
                 SelectionManager.SetSelectionsFromStartEnds(startEndStrings);
                 break;
             case FileManipulation.SelectWholeDoc:
-                var wholeSelStr = $"0,{Npp.editor.GetLength()}";
+                if (!Npp.TryGetLengthAsInt(out int len, false))
+                {
+                    messages.Add("FAIL: buffer was too long");
+                    return true;
+                }
+                var wholeSelStr = $"0,{len}";
                 messages.Add("select whole document");
                 SelectionManager.SetSelectionsFromStartEnds(new string[] { wholeSelStr });
                 break;
@@ -161,7 +166,11 @@ public static bool ExecuteFileManipulation(FileManipulation command, List<string
                 break;
             case FileManipulation.CompareText:
                 correctText = (string)args[0];
-                gotText = Npp.editor.GetText();
+                if (!Npp.TryGetText(out gotText, false))
+                {
+                    messages.Add("FAIL: buffer was too long");
+                    return true;
+                }
                 if (correctText != gotText)
                 {
                     messages.Add($"FAIL: expected text\r\n{correctText}\r\nGOT\r\n{gotText}");

diff --git a/NppCSharpPluginPack/Utils/Npp.cs b/NppCSharpPluginPack/Utils/Npp.cs
@@ -128,6 +128,49 @@ public static void TryCopyToClipboard(string text)
             Clipboard.SetText(text);
         }
 
+        private static bool stopShowingFileTooLongNotifications = false;
+
+        private static void WarnFileTooBig(bool showMessage)
+        {
+            if (showMessage && !stopShowingFileTooLongNotifications)
+                stopShowingFileTooLongNotifications = Translator.ShowTranslatedMessageBox(
+                    $"{Main.PluginName} cannot perform this plugin command on a file with more than 2147483647 bytes.\r\nDo you want to stop showing notifications when a file is too long?",
+                    $"File too long for {Main.PluginName}",
+                    MessageBoxButtons.YesNo, MessageBoxIcon.Warning) == DialogResult.Yes;
+        }
+
+        /// <summary>
+        /// if <see cref="IScintillaGateway.GetLength"/> returns a number greater than <see cref="int.MaxValue"/>, return false and set len to -1.<br></br>
+        /// Otherwise, return true and set len to the length of the document.<br></br>
+        /// If showMessageOnFail, show a message box warning the user that the command could not be executed.
+        /// </summary>
+        public static bool TryGetLengthAsInt(out int len, bool showMessageOnFail = true)
+        {
+            if (!editor.TryGetLengthAsInt(out len))
+            {
+                WarnFileTooBig(showMessageOnFail);
+                return false;
+            }
+            return true;
+        }
+
+        /// <summary>
+        /// if <see cref="IScintillaGateway.GetLength"/> returns a number greater than <see cref="int.MaxValue"/>, return false and set text to an empty string.<br></br>
+        /// Otherwise, return true and set text with <see cref="IScintillaGateway.TryGetText(out string, int)"/>.<br></br>
+        /// If showMessageOnFail, show a message box warning the user that the command could not be executed.
+        /// </summary>
+        public static bool TryGetText(out string text, bool showMessageOnFail = true, int length = -1)
+        {
+            if (length < 0 && !editor.TryGetLengthAsInt(out length))
+            {
+                text = "";
+                WarnFileTooBig(showMessageOnFail);
+                return false;
+            }
+            text = editor.GetText(length);
+            return true;
+        }
+
         public static string AssemblyVersionString()
         {
             string version = Assembly.GetExecutingAssembly().GetName().Version.ToString();

diff --git a/NppCSharpPluginPack/Utils/SelectionManager.cs b/NppCSharpPluginPack/Utils/SelectionManager.cs
@@ -1,6 +1,8 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Text.RegularExpressions;
+using System.Windows.Forms;
+using NppDemo.JSON_Tools;
 
 namespace NppDemo.Utils
 {
@@ -53,9 +55,19 @@ public static int[] ParseStartEnd(string startEnd)
             int ii = 0;
             Npp.editor.ClearSelections();
             var result = new List<(int start, int end)>();
+            var badPairs = new List<string>();
             foreach (string startEnd in startEnds)
             {
-                (int start, int end) = ParseStartEndAsTuple(startEnd);
+                int start, end;
+                try
+                {
+                    (start, end) = ParseStartEndAsTuple(startEnd);
+                }
+                catch
+                {
+                    badPairs.Add(startEnd);
+                    continue;
+                }
                 if (start > end)
                     (start, end) = (end, start);
                 result.Add((start, end));
@@ -70,6 +82,14 @@ public static int[] ParseStartEnd(string startEnd)
                     Npp.editor.AddSelection(start, end);
                 }
             }
+            if (badPairs.Count > 0)
+            {
+                string badPairJsonStr = "[" + string.Join(", ", badPairs.Select(x => JNode.StrToString(x, true))) + "]";
+                MessageBox.Show(
+                    $"While setting selections from start,end integer pairs, the following integer pairs were found that had a bad format:\r\n{badPairJsonStr}",
+                    "Bad start,end integer pairs when setting selections",
+                    MessageBoxButtons.OK, MessageBoxIcon.Error);
+            }
             return result;
         }
 

diff --git a/NppCSharpPluginPack/Utils/Translator.cs b/NppCSharpPluginPack/Utils/Translator.cs
@@ -194,10 +194,7 @@ private static void PropagateGlobalConstantsHelper(JNode node, Dictionary<string
         {
             if (node.value is string s && s.IndexOf('$') >= 0)
             {
-                string newValue = s;
-                foreach (KeyValuePair<string, string> kv in constants)
-                    newValue = newValue.Replace(kv.Key, kv.Value);
-                node.value = newValue;
+                node.value = PropagateConstantsToString(s, constants);
             }
             else if (node is JArray jarr)
             {
@@ -208,9 +205,30 @@ private static void PropagateGlobalConstantsHelper(JNode node, Dictionary<string
             {
                 foreach (JNode child in jobj.children.Values)
                     PropagateGlobalConstantsHelper(child, constants);
+                var keys = jobj.children.Keys.ToArray();
+                foreach (string key in keys)
+                {
+                    if (key.IndexOf('$') >= 0)
+                    {
+                        string newKey = PropagateConstantsToString(key, constants);
+                        if (newKey != key)
+                        {
+                            jobj[newKey] = jobj[key];
+                            jobj.children.Remove(key);
+                        }
+                    }
+                }
             }
         }
 
+        private static string PropagateConstantsToString(string s, Dictionary<string, string> constants)
+        {
+            string newValue = s;
+            foreach (KeyValuePair<string, string> kv in constants)
+                newValue = newValue.Replace(kv.Key, kv.Value);
+            return newValue;
+        }
+
         /// <summary>
         /// Translates menu item values (using <paramref name="menuItem"/> as the key in the <c>"menuItems"</c> field)
         /// </summary>

diff --git a/README.md b/README.md
@@ -68,7 +68,7 @@ If you are interested in helping users of your plugin who don't speak English, t
     - If no translation file was found, or if parsing failed, the default English will be used.
 5. If parsing was successful, `Translator.cs` will use the translation file as described below.
 
-To be clear, *the plugin may not be in the same language of the Notepad++ UI,* although I may change that in the future.
+To be clear, *the plugin may not be in the same language of the Notepad++ UI.* The steps described above represent my best effort to automatically translate the plugin into a language that the user will find useful, without requiring the user to select their language from a list of available languages in the settings form.
 
 To translate your plugin to another language, just look at [`english.json5` in the translations directory of this repo](https://github.com/molsonkiko/NppCSharpPluginPack/blob/main/translation/english.json5) and follow the instructions in that file.
 

diff --git a/most recent errors.txt b/most recent errors.txt
@@ -1,4 +1,4 @@
-Test results for CSharpPluginPack v0.0.3.7 on Notepad++ 8.6.4 64bit
+Test results for CSharpPluginPack v0.0.3.10 on Notepad++ 8.6.9 64bit
 NOTE: Ctrl-F (regular expressions *on*) for "Failed [1-9]\d*" to find all failed tests
 No tests failed
 =========================

diff --git a/translation/english.json5 b/translation/english.json5
@@ -15,11 +15,11 @@
 // 4. Use the JsonTools plugin (https://github.com/molsonkiko/JsonToolsNppPlugin) or another tool to make sure that this is a valid JSON with comments file.
 // 5. Close Notepad++ and reopen it, then look at the various $PluginName$ forms and menu items to see if the translation is satisfactory.
 {
-    // This lists a set of global constants that can appear in other values throughout this config file.
-    // For example, if the "$PluginName$" constant is "FooBar", and another value in this config file is "$PluginName$ settings",
-    //     that value would be changed to "FooBar settings" at the time the settings are loaded from file.
+    // This lists a set of global constants that can appear in other keys and values throughout this config file.
+    // For example, if the "$PluginName$" constant is "FooBar",
+    //     the object {"$PluginName$ settings": "About $PluginName$"}
+    //     would be translated to {"FooBar settings": "About FooBar"}
     // Constant names must match the regular expression "\A\$[a-zA-Z_]+\$\z", as shown below.
-    // Note to developers: Constant names are *not* replaced in keys of objects, only values.
     "$constants$": {
         "$PluginName$": "CSharpPluginPack"
     },
@@ -153,11 +153,12 @@
         "Could not open url in web browser": {"caption": "Could not open url in web browser", "text": "While attempting to open URL {0} in web browser, got exception\r\n{1}"},
         // ===== MessageBoxes in SettingsBase.cs =====
         // Note to developers: make sure to change these keys to match your plugin name!
-        "Unknown error while parsing CSharpPluginPack config file": {"caption": "Unknown error while parsing $PluginName$ config file", "text": "While parsing $PluginName$ config file, expected setting \"{0}\" to be type {1}, but got an error.\r\nThat setting was set to its default value of {2}.\r\nThe given value {3} could not be converted for an unknown reason."},
-        "Error while parsing CSharpPluginPack config file": {"caption": "Error while parsing $PluginName$ config file", "text": "While parsing $PluginName$ config file, expected setting \"{0}\" to be type {1}, but got an error.\r\nThat setting was set to its default value of {2}.\r\nThe given value {3} raised the following error:\r\n{4}"},
+        "Unknown error while parsing $PluginName$ config file": {"caption": "Unknown error while parsing $PluginName$ config file", "text": "While parsing $PluginName$ config file, expected setting \"{0}\" to be type {1}, but got an error.\r\nThat setting was set to its default value of {2}.\r\nThe given value {3} could not be converted for an unknown reason."},
+        "Error while parsing $PluginName$ config file": {"caption": "Error while parsing $PluginName$ config file", "text": "While parsing $PluginName$ config file, expected setting \"{0}\" to be type {1}, but got an error.\r\nThat setting was set to its default value of {2}.\r\nThe given value {3} raised the following error:\r\n{4}"},
         "Invalid value for setting {0}": {"caption": "Invalid value for setting {0}", "text": "Could not change setting {0} to value {1}, so it will remain set as {2}.\r\nGot the following exception:\r\n{3}"},
         // ===== MessageBoxes in Npp.cs =====
         "Nothing to copy to clipboard": {"caption": "Nothing to copy to clipboard", "text": "Couldn't find anything to copy to the clipboard"},
+        "File too long for $PluginName$": {"caption": "File too long for $PluginName$", "text": "$PluginName$ cannot perform this plugin command on a file with more than 2147483647 bytes.\r\nDo you want to stop showing notifications when a file is too long?"},
         // ===== MessageBoxes in TestRunner.cs =====
         "Do you want to run tests?": {"caption": "Do you want to run tests?", "text": "Running tests can potentially overwrite the contents of your clipboard. Do you still want to run tests?"}
     }