From 291259798f2a67446cc2c1177e6ab62f16aecc93 Mon Sep 17 00:00:00 2001
From: datbth <datbth@gmail.com>
Date: Wed, 23 Mar 2022 23:30:47 +0700
Subject: [PATCH] Validate worksheet name using Libxlsxwriter

---
 Gemfile.lock                       |  3 +++
 lib/fast_excel.rb                  | 10 ++++++----
 lib/fast_excel/binding/workbook.rb |  6 +++---
 test/validations_test.rb           | 15 +++++++++++++--
 4 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index 6ddda65..00ecd9e 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -42,6 +42,8 @@ GEM
       ruby-progressbar
     nokogiri (1.13.3-x86_64-darwin)
       racc (~> 1.4)
+    nokogiri (1.13.3-x86_64-linux)
+      racc (~> 1.4)
     racc (1.6.0)
     rake (13.0.6)
     ruby-progressbar (1.11.0)
@@ -57,6 +59,7 @@ GEM
 
 PLATFORMS
   x86_64-darwin-20
+  x86_64-linux
 
 DEPENDENCIES
   axlsx!
diff --git a/lib/fast_excel.rb b/lib/fast_excel.rb
index 8cbf18a..0b717f0 100644
--- a/lib/fast_excel.rb
+++ b/lib/fast_excel.rb
@@ -125,6 +125,7 @@ def self.print_ffi_obj(value)
   end
 
 
+  ERROR_ENUM = Libxlsxwriter.enum_type(:error)
   COLOR_ENUM = Libxlsxwriter.enum_type(:defined_colors)
   EXTRA_COLORS = {
     alice_blue: 0xF0F8FF,
@@ -366,10 +367,11 @@ def number_format(pattern)
 
     def add_worksheet(sheetname = nil)
       if !sheetname.nil?
-        if sheetname.length > Libxlsxwriter::SHEETNAME_MAX
-          raise ArgumentError, "Worksheet name '#{sheetname}' exceeds Excel's limit of #{Libxlsxwriter::SHEETNAME_MAX} characters"
-        elsif @sheet_names.include?(sheetname)
-          raise ArgumentError, "Worksheet name '#{sheetname}' is already in use"
+        error = validate_worksheet_name(sheetname)
+        if error != :no_error
+          error_code = ERROR_ENUM.find(error)
+          error_str = error_code ? Libxlsxwriter.strerror(error_code) : ''
+          raise ArgumentError, "Invalid worksheet name '#{sheetname}': (#{error_code} - #{error}) #{error_str}"
         end
       end
 
diff --git a/lib/fast_excel/binding/workbook.rb b/lib/fast_excel/binding/workbook.rb
index 96fa9f2..fc7e184 100644
--- a/lib/fast_excel/binding/workbook.rb
+++ b/lib/fast_excel/binding/workbook.rb
@@ -147,7 +147,7 @@ def get_worksheet_by_name(name)
     # @param [String] sheetname 
     # @return [Symbol from _enum_error_] 
     def validate_worksheet_name(sheetname)
-      Libxlsxwriter.workbook_validate_worksheet_name(self, sheetname)
+      Libxlsxwriter.workbook_validate_sheet_name(self, sheetname)
     end
   
     # @return [nil] 
@@ -319,12 +319,12 @@ class Workbook < FFI::Struct
   # @scope class
   attach_function :workbook_get_worksheet_by_name, :workbook_get_worksheet_by_name, [Workbook, :string], Worksheet
 
-  # @method workbook_validate_worksheet_name(workbook, sheetname)
+  # @method workbook_validate_sheet_name(workbook, sheetname)
   # @param [Workbook] workbook 
   # @param [String] sheetname 
   # @return [Symbol from _enum_error_] 
   # @scope class
-  attach_function :workbook_validate_worksheet_name, :workbook_validate_worksheet_name, [Workbook, :string], :error
+  attach_function :workbook_validate_sheet_name, :workbook_validate_sheet_name, [Workbook, :string], :error
 
   # @method workbook_free(workbook)
   # @param [Workbook] workbook 
diff --git a/test/validations_test.rb b/test/validations_test.rb
index 8abedf3..cdb6d89 100644
--- a/test/validations_test.rb
+++ b/test/validations_test.rb
@@ -11,7 +11,7 @@
     end
 
     assert_equal(ArgumentError, error.class)
-    assert_equal("Worksheet name 'Payments Report' is already in use", error.message)
+    assert_equal("Invalid worksheet name 'Payments Report': (16 - error_sheetname_already_used) Worksheet name is already in use.", error.message)
   end
 
   it "should not raise error when worksheet name is null" do
@@ -33,7 +33,7 @@
     end
 
     assert_equal(ArgumentError, error.class)
-    assert_equal("Worksheet name 'ABCDEFGHIJKLMNOPQRSTUVWXYZ012345' exceeds Excel's limit of 31 characters", error.message)
+    assert_equal("Invalid worksheet name 'ABCDEFGHIJKLMNOPQRSTUVWXYZ012345': (13 - error_sheetname_length_exceeded) Worksheet name exceeds Excel's limit of 31 characters.", error.message)
   end
 
   it "should not raise error when the sheet name is at maximum length" do
@@ -44,4 +44,15 @@
 
     assert_equal("ABCDEFGHIJKLMNOPQRSTUVWXYZ01234", worksheet[:name])
   end
+
+  it "should validate using Libxlsxwriter validation" do
+    workbook = FastExcel.open(constant_memory: true)
+    error = assert_raises do
+      worksheet = workbook.add_worksheet('a?')
+      worksheet.write_value(1, 1, 'a') # without the validation, this method will crash the process
+    end
+
+    assert_equal(ArgumentError, error.class)
+    assert_equal("Invalid worksheet name 'a?': (14 - error_invalid_sheetname_character) Worksheet name cannot contain invalid characters: '[ ] : * ? / \\'", error.message)
+  end
 end