CSVLint causes Notepad++ crash when attempting to open file with more than 2**31 - 1 bytes

[molsonkiko]

To replicate

1. Open any file with more than 2**31 - 1 bytes (i.e., the maximum size of a C# string, hereafter referred to as int.MaxValue).
2. Notepad++ will crash

Expected behavior

Obviously I expect no crash. I would also expect CSVLint to explain to the user that the file is too large for CSVLint to open whenever they manually run a plugin command.

Debug info

Notepad++ v8.6.9   (64-bit)
Build time : Jul 12 2024 - 05:09:25
Path : C:\Program Files\Notepad++\notepad++.exe
Command Line : 
Admin mode : OFF
Local Conf mode : OFF
Cloud Config : OFF
Periodic Backup : ON
OS Name : Windows 10 Home (64-bit)
OS Version : 22H2
OS Build : 19045.4651
Current ANSI codepage : 1252
Plugins : 
    ColumnsPlusPlus (1.1.1)
    ComparePlus (1.1)
    CSharpPluginPack (0.0.3.9)
    CSVLint (0.4.6.6)
    EnhanceAnyLexer (1.1.3)
    HTMLTag (1.4.3.1)
    HugeFiles (0.4.1)
    JsonTools (8.0.0.17)
    mimeTools (3.1)
    NavigateTo (2.7)
    NppConverter (4.6)
    NppExport (0.4)
    NppLspClient (0.0.21)
    PythonScript (3.0.16)
    XMLTools (3.1.1.13)

Proposed solution

The sneaky problem with the plugin infrastructure we use is that you can't even attempt to get the length of a file with length greater than int.MaxValue if the ISCintillaGateway.GetLength() method returns an int. The obvious solution is to have the method return a long, but it's annoying to do bounds checking on the long every time, so the best solution is to have a helper method.

I'm not going to submit a PR because I don't have the .NET Framework 4.0 targeting pack installed and I don't feel like installing it, but you can see how I changed JsonTools to fix this issue.

In short, you want to do the following:

1. Change ScintillaGateway and IScintillaGateway so that the GetLength() method returns a long:

   public long GetLength()
   {
       return Win32.SendMessage(scintilla, SciMsg.SCI_GETLENGTH, (IntPtr)Unused, (IntPtr)Unused).ToInt64();
   }

2. add a helper method (and related global variable) to your Main class that does the bounds checking and (optionally) warns the user if the file is too big:

   private static bool stopShowingFileTooLongNotifications = false;
   /// <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)
   {
       long result = editor.GetLength();
       if (result > int.MaxValue)
       {
           len = -1;
           if (!stopShowingFileTooLongNotifications && showMessageOnFail)
           {
               stopShowingFileTooLongNotifications = MessageBox.Show(
                   "CSVLint 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 CSVLint",
                   MessageBoxButtons.YesNo, MessageBoxIcon.Warning) == DialogResult.Yes;
   
           }
           return false;
       }
       len = (int)result;
       return true;
   }

3. Refactor methods that look like this:

   // This is the *BAD* old version
   public static void MyMethod()
   {
       int len = editor.GetLength();
       // do something with len
   }

4. into methods that look like this:

   // This is the *GOOD* new version
   public static void MyMethod()
   {
       if (!Main.TryGetLengthAsInt(out int len))
           return;
       // do something with len
   }

[BdR76]

Thanks for posting the issue. I think Notepad++ runs into other issues as well when opening such large >2GB files.

I'll look into this when I have the time. For now, I've updated the generate_data.py script to generate a cardio.txt file of 2GB, if you change the TOTAL_LINES variable.

[molsonkiko]

@BdR76

I decided to take a crack at solving this problem myself, and this commit is the result. There are a fair number of lines of code changed, but the basic idea is quite simple: check for file length before creating a StreamReader, and (optionally) notify the user if the file was too long. Because ScintillaStreams.StreamAllText potentially returning null would have made it necessary to do a null check after every call, I opted to turn it int a Dictionary.TryGetValue-style function, because I think that's a cleaner idiom for this sort of problem.

One thing to note: I didn't actually test the code as it was committed; rather, I tested it all with <TargetFrameworkVersion>v4.8</TargetFrameworkVersion> in the csproj because I didn't want to install the .NET Framework 4.0 targeting pack, and then changed the framework version back to 4.0 just before committing, after I'd done some (very basic) testing in 4.8 and verified that it works.

You may feel that this is too much code, and I'd be very open to suggestions for improvement, but as far as I can tell it's just about the smallest change that would be sufficient to eliminate all possibility of crashes on too-large files.

[BdR76]

@molsonkiko Thanks for looking into this issue 👏 I've looked over the changes and it looks good to me. Could you submit the PR? I'll it test some more and put it into a new release when I have the time.

(btw I know you looked at it before, but do you maybe also have a clue how to solve the transparent cursor line interfering with the ComparePlus? See #68 )

[molsonkiko]

@BdR76

#94 should address this issue. I added a couple more commits to ensure that all plugin commands warn the user if the file is too large, rather than failing silently.

With regards to #68, I still don't understand the underlying issues (in your plugin or ComparePlus), but I may take a crack at it at some point in the future.