Merge pull request #508 from softmarch/issue-494-shared-string

Fix throwing exception when duplicate shared strings occur.

include/xlnt/workbook/workbook.hpp

@@ -767,11 +767,6 @@ public:
     /// </summary>
     std::size_t add_shared_string(const rich_text &shared, bool allow_duplicates = false);
 
-    /// <summary>
-    /// Returns a reference to the shared string ordered by id
-    /// </summary>
-    const std::map<std::size_t, rich_text> &shared_strings_by_id() const;
-
     /// <summary>
     /// Returns a reference to the shared string related to the specified index
     /// </summary>
@@ -781,13 +776,13 @@ public:
     /// Returns a reference to the shared strings being used by cells
     /// in this workbook.
     /// </summary>
-    std::unordered_map<rich_text, std::size_t, rich_text_hash> &shared_strings();
+    std::vector<rich_text> &shared_strings();
 
     /// <summary>
     /// Returns a reference to the shared strings being used by cells
     /// in this workbook.
     /// </summary>
-    const std::unordered_map<rich_text, std::size_t, rich_text_hash> &shared_strings() const;
+    const std::vector<rich_text> &shared_strings() const;
 
     // Thumbnail
 

source/detail/implementations/workbook_impl.hpp

@@ -120,7 +120,7 @@ struct workbook_impl
 
     std::list<worksheet_impl> worksheets_;
     std::unordered_map<rich_text, std::size_t, rich_text_hash> shared_strings_ids_;
-    std::map<std::size_t, rich_text> shared_strings_values_;
+    std::vector<rich_text> shared_strings_values_;
 
     optional<stylesheet> stylesheet_;
 

source/detail/serialization/xlsx_consumer.cpp

@@ -2098,8 +2098,8 @@ void xlsx_consumer::read_shared_string_table()
     {
         expect_start_element(qn("spreadsheetml", "si"), xml::content::complex);
         auto rt = read_rich_text(qn("spreadsheetml", "si"));
-		//by reading in it can happen we have similar strings from modified excel worksheets...
+        // by reading in it can happen we have similar strings from modified excel worksheets
-		//so allow to add duplicates
+        // so allow to add duplicates
         target_.add_shared_string(rt, true);
         expect_end_element(qn("spreadsheetml", "si"));
     }

source/detail/serialization/xlsx_producer.cpp

@@ -944,12 +944,12 @@ void xlsx_producer::write_shared_string_table(const relationship & /*rel*/)
     }
 
     write_attribute("count", string_count);
-    write_attribute("uniqueCount", source_.shared_strings_by_id().size());
+    write_attribute("uniqueCount", source_.shared_strings().size());
 
-    for (const auto &string : source_.shared_strings_by_id())
+    for (const auto &text : source_.shared_strings())
     {
         write_start_element(xmlns, "si");
-        write_rich_text(xmlns, string.second);
+        write_rich_text(xmlns, text);
         write_end_element(xmlns, "si");
     }
 

source/workbook/workbook.cpp

@@ -1358,32 +1358,25 @@ const manifest &workbook::manifest() const
     return d_->manifest_;
 }
 
-const std::map<std::size_t, rich_text> &workbook::shared_strings_by_id() const
-{
-    return d_->shared_strings_values_;
-}
-
 const rich_text &workbook::shared_strings(std::size_t index) const
 {
-    auto it = d_->shared_strings_values_.find(index);
+    if (index < d_->shared_strings_values_.size())
-
-    if (it != d_->shared_strings_values_.end())
     {
-        return it->second;
+        return d_->shared_strings_values_.at(index);
     }
 
     static rich_text empty;
     return empty;
 }
 
-std::unordered_map<rich_text, std::size_t, rich_text_hash> &workbook::shared_strings()
+std::vector<rich_text> &workbook::shared_strings()
 {
-    return d_->shared_strings_ids_;
+    return d_->shared_strings_values_;
 }
 
-const std::unordered_map<rich_text, std::size_t, rich_text_hash> &workbook::shared_strings() const
+const std::vector<rich_text> &workbook::shared_strings() const
 {
-    return d_->shared_strings_ids_;
+    return d_->shared_strings_values_;
 }
 
 std::size_t workbook::add_shared_string(const rich_text &shared, bool allow_duplicates)
@@ -1400,19 +1393,18 @@ std::size_t workbook::add_shared_string(const rich_text &shared, bool allow_dupl
         }
     }
 
- 	//it can happen that similar strings are more then onetime in the shared stringtable (Excel bugfix?)
+    // it can happen that similar strings are more then onetime in the shared stringtable (Excel bugfix?)
-	//shared_strings_values map should start on position 0
+    // shared_strings_values map should start on position 0
-	auto sz = d_->shared_strings_values_.size();
+    auto sz = d_->shared_strings_values_.size();
     if (d_->shared_strings_values_.count(sz) > 0)
     {
         // something went wrong!
         throw invalid_file("Error in shared string table!");
-	}
+	  }
 
     d_->shared_strings_values_[sz] = shared;
     d_->shared_strings_ids_[shared] = sz;
 
-
     return sz;
 }
 

tests/workbook/workbook_test_suite.cpp

@@ -68,6 +68,7 @@ public:
         register_test(test_load_file);
         register_test(test_Issue279);
         register_test(test_Issue353);
+        register_test(test_Issue494);
     }
 
     void test_active_sheet()
@@ -509,5 +510,14 @@ public:
         xlnt_assert_equals(ws.row_properties(1).spans.get(), "1:8");
         xlnt_assert_equals(ws.row_properties(17).spans.get(), "2:7");
     }
+
+    void test_Issue494()
+    {
+        xlnt::workbook wb;
+        wb.load(path_helper::test_file("Issue494_shared_string.xlsx"));
+        auto ws = wb.active_sheet();
+        xlnt_assert_equals(ws.cell(2, 1).to_string(), "V1.00");
+        xlnt_assert_equals(ws.cell(2, 2).to_string(), "V1.00");
+    }
 };
 static workbook_test_suite x;