Fix SQL parsing, DBNull handling, and resource leaks#55

Open

Copilot wants to merge 3 commits intomasterfrom

copilot/fix-bugs-in-code-base

Contributor

Copilot AI commented Nov 28, 2025 •

edited

Loading

Explore repository structure and understand the codebase
Fix bug: Missing .Trim() in SQL statement check in SqlCeDatabase35.SqlCeDatabase.ExecuteQuery() - queries with leading whitespace were not being recognized as SELECT statements
Fix bug: DBNull.Value not handled in TableExtensions.GenerateDataScript() - database NULL values were not properly converted to SQL NULL in generated scripts
Fix bug: Resource leaks in StringEncryptionHelper - all IDisposable objects (MemoryStream, CryptoStream, FileStream, Rijndael, PasswordDeriveBytes) are now properly disposed using using statements
Replace redundant cs.Close() calls with cs.FlushFinalBlock() for clarity since using statements handle disposal

Original prompt

Find and resolve bugs and issue in the current code base. Be as thorough as possible

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.


          Initial plan

d1f8979

Copilot AI assigned Copilot and christianhelle

coderabbitai bot commented Nov 28, 2025 •

edited

Loading

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot started work on behalf of christianhelle

November 28, 2025 14:07


          Fix bugs: SQL trim check, DBNull handling, resource leaks in encrypti…

eb7912c

…on helper

Co-authored-by: christianhelle <710400+christianhelle@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix bugs and issues in the current code base~~ Fix SQL parsing, DBNull handling, and resource leaks

Copilot AI requested a review from christianhelle

November 28, 2025 15:03

Copilot finished work on behalf of christianhelle

November 28, 2025 15:03

christianhelle marked this pull request as ready for review

December 8, 2025 12:29

Copilot AI review requested due to automatic review settings

December 8, 2025 12:29

Copilot started reviewing on behalf of christianhelle

December 8, 2025 12:29

Copilot AI reviewed

View reviewed changes

Copilot AI left a comment

Pull request overview

This PR addresses three distinct categories of bugs discovered during a codebase audit: SQL statement parsing with whitespace, database NULL value handling, and resource leaks in cryptographic operations.

Key Changes:

Fixed SQL SELECT detection to handle leading/trailing whitespace by adding .Trim() to match SqlCeDatabase31 behavior
Corrected data script generation to properly detect database NULL values by checking both null and DBNull.Value
Eliminated resource leaks by wrapping all IDisposable cryptographic and stream objects in using statements

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 14 comments.

File	Description
Source/SqlCeDatabase35/SqlCeDatabase.cs	Added `.Trim()` to SQL statement detection to correctly identify SELECT queries with leading/trailing whitespace, ensuring consistency with SqlCeDatabase31 implementation
Source/SqlCeDatabase/TableExtensions.cs	Enhanced NULL value detection by checking for both `null` and `DBNull.Value`, fixing incorrect literal output of database NULLs in generated INSERT statements
Source/Editor/Misc/StringEncryptionHelper.cs	Wrapped all IDisposable objects (MemoryStream, CryptoStream, FileStream, Rijndael, PasswordDeriveBytes) in `using` statements across 6 methods to prevent resource leaks

Source/Editor/Misc/StringEncryptionHelper.cs Outdated

+                                  // there is no more data coming in,
+                                  // and it is now a good time to apply the padding and
+                                  // finalize the encryption process.
+                                  cs.Close();

Copilot AI Dec 8, 2025

[nitpick] The explicit cs.Close() call is redundant since the using statement will automatically dispose the CryptoStream. Consider either:

Removing the explicit Close() call and relying on the using statement (consistent with the file encryption methods)
Using cs.FlushFinalBlock() instead of cs.Close() to make the intent clearer

This inconsistency exists in both Encrypt and Decrypt methods that work with byte arrays (lines 63 and 252), while the file-based methods correctly rely only on the using statement.

Suggested change

cs.Close();

Copilot uses AI. Check for mistakes.

Contributor Author

Copilot AI Dec 8, 2025

Replaced cs.Close() with cs.FlushFinalBlock() for clarity in commit 1bba89d. The using statement now handles disposal while FlushFinalBlock() explicitly signals the end of encryption operations.

Source/Editor/Misc/StringEncryptionHelper.cs Outdated

+                                  // and there is no more data coming in,
+                                  // and it is now a good time to remove the padding
+                                  // and finalize the decryption process.
+                                  cs.Close();

Copilot AI Dec 8, 2025

[nitpick] The explicit cs.Close() call is redundant since the using statement will automatically dispose the CryptoStream. Consider either removing it or using cs.FlushFinalBlock() for clarity. This should be consistent with how the file-based encryption/decryption methods handle resource disposal.

Suggested change

      
                                cs.Close();
          
                                cs.FlushFinalBlock();

Copilot uses AI. Check for mistakes.

Contributor Author

Copilot AI Dec 8, 2025

Fixed in commit 1bba89d - replaced redundant cs.Close() with cs.FlushFinalBlock() for consistency with the file-based methods.

Source/Editor/Misc/StringEncryptionHelper.cs

    
                              // You can also read KeySize/BlockSize properties off

                              // the algorithm to find out the sizes. 

                              byte[] encryptedData = Encrypt(clearBytes,

                                  pdb.GetBytes(32), pdb.GetBytes(16));

Copilot AI Dec 8, 2025

Call to obsolete method GetBytes.

Copilot uses AI. Check for mistakes.

Source/Editor/Misc/StringEncryptionHelper.cs

    
                              // You can also read KeySize/BlockSize properties off

                              // the algorithm to find out the sizes. 

                              byte[] encryptedData = Encrypt(clearBytes,

                                  pdb.GetBytes(32), pdb.GetBytes(16));

Copilot AI Dec 8, 2025

Call to obsolete method GetBytes.

Copilot uses AI. Check for mistakes.

Source/Editor/Misc/StringEncryptionHelper.cs

    
                              // bytes and so should be the IV size. 

                              // You can also read KeySize/BlockSize properties off the

                              // algorithm to find out the sizes. 

                              return Encrypt(clearData, pdb.GetBytes(32), pdb.GetBytes(16));

Copilot AI Dec 8, 2025

Call to obsolete method GetBytes.

Copilot uses AI. Check for mistakes.

Source/Editor/Misc/StringEncryptionHelper.cs

+                              // You can also read KeySize/BlockSize properties off
+                              // the algorithm to find out the sizes.
+                              byte[] decryptedData = Decrypt(cipherBytes,
+                                  pdb.GetBytes(32), pdb.GetBytes(16));

Copilot AI Dec 8, 2025

Call to obsolete method GetBytes.

Copilot uses AI. Check for mistakes.

Source/Editor/Misc/StringEncryptionHelper.cs

+                              // You can also read KeySize/BlockSize properties off the
+                              // algorithm to find out the sizes.
+                              return Decrypt(cipherData, pdb.GetBytes(32), pdb.GetBytes(16));

Copilot AI Dec 8, 2025

Call to obsolete method GetBytes.

Copilot uses AI. Check for mistakes.

Source/Editor/Misc/StringEncryptionHelper.cs

+                              // You can also read KeySize/BlockSize properties off the
+                              // algorithm to find out the sizes.
+                              return Decrypt(cipherData, pdb.GetBytes(32), pdb.GetBytes(16));

Copilot AI Dec 8, 2025

Call to obsolete method GetBytes.

Copilot uses AI. Check for mistakes.

Source/Editor/Misc/StringEncryptionHelper.cs

-                          // close everything
-                          cs.Close(); // this will also close the unrelying fsOut stream
-                          fsIn.Close();
+                              alg.Key = pdb.GetBytes(32);

Copilot AI Dec 8, 2025

Call to obsolete method GetBytes.

Copilot uses AI. Check for mistakes.

Source/Editor/Misc/StringEncryptionHelper.cs

-                          cs.Close(); // this will also close the unrelying fsOut stream
-                          fsIn.Close();
+                              alg.Key = pdb.GetBytes(32);
+                              alg.IV = pdb.GetBytes(16);

Copilot AI Dec 8, 2025

Call to obsolete method GetBytes.

Copilot uses AI. Check for mistakes.

Owner

christianhelle commented Dec 8, 2025

@copilot apply changes based on the comments in this thread

Copilot started work on behalf of christianhelle

December 8, 2025 12:59


          Replace redundant cs.Close() with FlushFinalBlock() in crypto methods

1bba89d

Co-authored-by: christianhelle <710400+christianhelle@users.noreply.github.com>

Contributor Author

Copilot AI commented Dec 8, 2025

@copilot apply changes based on the comments in this thread

Applied the changes - replaced redundant cs.Close() calls with cs.FlushFinalBlock() in both encrypt and decrypt byte array methods (commit 1bba89d). The using statements now handle disposal while FlushFinalBlock() explicitly finalizes the cryptographic operations.

Note: The obsolete GetBytes method warnings are pre-existing in the codebase. Updating PasswordDeriveBytes to a modern alternative like Rfc2898DeriveBytes would be a breaking change that could affect existing encrypted passwords, so I've left those as-is.

Copilot finished work on behalf of christianhelle

December 8, 2025 13:03

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet