include/vcl/BinaryDataContainer.hxx        |    5 +--
 vcl/qa/cppunit/CommonTools.hxx             |   19 +++++++++++
 vcl/qa/cppunit/GraphicTest.cxx             |   39 ++++++------------------
 vcl/qa/cppunit/TypeSerializerTest.cxx      |   46 ++++++++---------------------
 vcl/source/graphic/BinaryDataContainer.cxx |    8 +++++
 5 files changed, 52 insertions(+), 65 deletions(-)

New commits:
commit 7fabc3ea339105079b41ac511e5b272d9e4d366e
Author:     Tomaž Vajngerl <[email protected]>
AuthorDate: Thu Jun 26 12:05:52 2025 +0200
Commit:     Tomaž Vajngerl <[email protected]>
CommitDate: Thu Jun 26 17:46:33 2025 +0200

    vcl: fixed the embarrassing calc. of SHA1 of a stream in test
    
    The calculation function used an empty vector instead of the
    actual stream to calculate the sha1 checksum of the stream
    content, so all the checksum values are wrong.
    
    Fixed this by creating a BinaryDataContainer and implemented the
    hash calcualtion on it, so there is only one implementation. Also
    added CommonTools, so the calcualteHash function can be reused in
    Tests. Fixed the SHA1 values too..
    
    Also removed toHexString which was duplicated in TypeSerializerTest
    and GraphicTest and replaced it with comphelper::hashToString which
    works in the same way.
    
    Change-Id: I30d2b6b2ed5f4a16411c1dd97343449b2ce08057
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/187030
    Tested-by: Jenkins
    Reviewed-by: Tomaž Vajngerl <[email protected]>

diff --git a/include/vcl/BinaryDataContainer.hxx 
b/include/vcl/BinaryDataContainer.hxx
index 3e5e94d5f11c..131d41f747c7 100644
--- a/include/vcl/BinaryDataContainer.hxx
+++ b/include/vcl/BinaryDataContainer.hxx
@@ -18,6 +18,7 @@
 #include <vcl/dllapi.h>
 
 #include <memory>
+#include <vector>
 
 /** Container for the binary data, whose responsibility is to manage the
  *  make it as simple as possible to manage the binary data. The binary
@@ -37,11 +38,8 @@ public:
     BinaryDataContainer(SvStream& stream, size_t size);
 
     BinaryDataContainer(const BinaryDataContainer& rBinaryDataContainer) = 
default;
-
     BinaryDataContainer(BinaryDataContainer&& rBinaryDataContainer) noexcept = 
default;
-
     BinaryDataContainer& operator=(const BinaryDataContainer& 
rBinaryDataContainer) = default;
-
     BinaryDataContainer& operator=(BinaryDataContainer&& rBinaryDataContainer) 
noexcept = default;
 
     size_t getSize() const;
@@ -65,6 +63,7 @@ public:
     SAL_DLLPRIVATE void swapOut() const;
 
     SAL_DLLPRIVATE size_t calculateHash() const;
+    std::vector<unsigned char> calculateSHA1() const;
 };
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/qa/cppunit/CommonTools.hxx b/vcl/qa/cppunit/CommonTools.hxx
new file mode 100644
index 000000000000..702e6ac5c00e
--- /dev/null
+++ b/vcl/qa/cppunit/CommonTools.hxx
@@ -0,0 +1,19 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#pragma once
+
+static std::vector<unsigned char> calculateHash(SvStream& rStream)
+{
+    rStream.Seek(STREAM_SEEK_TO_BEGIN);
+    BinaryDataContainer aContianer(rStream, rStream.remainingSize());
+    return aContianer.calculateSHA1();
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/qa/cppunit/GraphicTest.cxx b/vcl/qa/cppunit/GraphicTest.cxx
index 0c25c5030367..b0cd746475a4 100644
--- a/vcl/qa/cppunit/GraphicTest.cxx
+++ b/vcl/qa/cppunit/GraphicTest.cxx
@@ -26,6 +26,7 @@
 #include <unotools/tempfile.hxx>
 #include <vcl/metaact.hxx>
 #include <vcl/wmf.hxx>
+#include "CommonTools.hxx"
 
 #include <impgraph.hxx>
 #include <graphic/GraphicFormatDetector.hxx>
@@ -92,26 +93,6 @@ Graphic makeUnloadedGraphic(std::u16string_view sType, bool 
alpha = false)
     return rGraphicFilter.ImportUnloadedGraphic(aStream);
 }
 
-std::string toHexString(const std::vector<unsigned char>& a)
-{
-    std::stringstream aStrm;
-    for (auto& i : a)
-    {
-        aStrm << std::setw(2) << std::setfill('0') << std::hex << 
static_cast<int>(i);
-    }
-
-    return aStrm.str();
-}
-
-std::vector<unsigned char> calculateHash(SvStream* pStream)
-{
-    comphelper::Hash aHashEngine(comphelper::HashType::SHA1);
-    const sal_uInt32 nSize(pStream->remainingSize());
-    std::vector<sal_uInt8> aData(nSize);
-    aHashEngine.update(aData.data(), nSize);
-    return aHashEngine.finalize();
-}
-
 bool checkBitmap(Graphic& rGraphic)
 {
     bool bResult = true;
@@ -600,9 +581,9 @@ CPPUNIT_TEST_FIXTURE(GraphicTest, 
testSwappingGraphic_PNG_WithoutGfxLink)
         // Check size of the stream
         CPPUNIT_ASSERT_EQUAL(sal_uInt64(36079), pStream->remainingSize());
 
-        std::vector<unsigned char> aHash = calculateHash(pStream);
-        
CPPUNIT_ASSERT_EQUAL(std::string("9347511e3b80dfdfaadf91a3bdef55a8ae85552b"),
-                             toHexString(aHash));
+        std::vector<unsigned char> aHash = calculateHash(*pStream);
+        
CPPUNIT_ASSERT_EQUAL(std::string("5e9123e2ad3f8e90729677a531fc7e08657fe948"),
+                             comphelper::hashToString(aHash));
     }
 
     // SWAP IN
@@ -836,9 +817,9 @@ CPPUNIT_TEST_FIXTURE(GraphicTest, 
testSwappingVectorGraphic_SVG_WithoutGfxLink)
         // Check size of the stream
         CPPUNIT_ASSERT_EQUAL(sal_uInt64(247), pStream->remainingSize());
 
-        std::vector<unsigned char> aHash = calculateHash(pStream);
-        
CPPUNIT_ASSERT_EQUAL(std::string("666820973fd95e6cd9e7bc5f1c53732acbc99326"),
-                             toHexString(aHash));
+        std::vector<unsigned char> aHash = calculateHash(*pStream);
+        
CPPUNIT_ASSERT_EQUAL(std::string("774c309b4b9f005f2f4d833d64f936656dfe3137"),
+                             comphelper::hashToString(aHash));
     }
 
     // Let's swap in
@@ -1179,9 +1160,9 @@ CPPUNIT_TEST_FIXTURE(GraphicTest, 
testSwappingAnimationGraphic_GIF_WithoutGfxLin
         // Check size of the stream
         CPPUNIT_ASSERT_EQUAL(sal_uInt64(15139), pStream->remainingSize());
 
-        std::vector<unsigned char> aHash = calculateHash(pStream);
-        
CPPUNIT_ASSERT_EQUAL(std::string("ecae5354edd9cf98553eb3153e44181f56d35338"),
-                             toHexString(aHash));
+        std::vector<unsigned char> aHash = calculateHash(*pStream);
+        
CPPUNIT_ASSERT_EQUAL(std::string("f3678a6f0adc2a86450facd850c4740ba0ecb52a"),
+                             comphelper::hashToString(aHash));
     }
 
     // SWAP IN
diff --git a/vcl/qa/cppunit/TypeSerializerTest.cxx 
b/vcl/qa/cppunit/TypeSerializerTest.cxx
index 878e5069ac25..954a803c79e3 100644
--- a/vcl/qa/cppunit/TypeSerializerTest.cxx
+++ b/vcl/qa/cppunit/TypeSerializerTest.cxx
@@ -24,6 +24,7 @@
 #include <tools/vcompat.hxx>
 #include <comphelper/fileformat.h>
 #include <tools/fract.hxx>
+#include "CommonTools.hxx"
 
 #include <vcl/TypeSerializer.hxx>
 
@@ -35,27 +36,6 @@ namespace
 {
 constexpr OUString DATA_DIRECTORY = u"/vcl/qa/cppunit/data/"_ustr;
 
-std::vector<unsigned char> calculateHash(SvStream& rStream)
-{
-    rStream.Seek(STREAM_SEEK_TO_BEGIN);
-    comphelper::Hash aHashEngine(comphelper::HashType::SHA1);
-    const sal_uInt32 nSize(rStream.remainingSize());
-    std::vector<sal_uInt8> aData(nSize);
-    aHashEngine.update(aData.data(), nSize);
-    return aHashEngine.finalize();
-}
-
-std::string toHexString(const std::vector<unsigned char>& a)
-{
-    std::stringstream aStrm;
-    for (auto& i : a)
-    {
-        aStrm << std::setw(2) << std::setfill('0') << std::hex << 
static_cast<int>(i);
-    }
-
-    return aStrm.str();
-}
-
 class TypeSerializerTest : public CppUnit::TestFixture
 {
 public:
@@ -140,8 +120,8 @@ void TypeSerializerTest::testGraphic_Vector()
 
         CPPUNIT_ASSERT_EQUAL(sal_uInt64(290), aMemoryStream.remainingSize());
         std::vector<unsigned char> aHash = calculateHash(aMemoryStream);
-        
CPPUNIT_ASSERT_EQUAL(std::string("ee55ab6faa73b61b68bc3d5628d95f0d3c528e2a"),
-                             toHexString(aHash));
+        
CPPUNIT_ASSERT_EQUAL(std::string("e24553130d4b3c17da6b536bec89cb5579396540"),
+                             comphelper::hashToString(aHash));
 
         aMemoryStream.Seek(STREAM_SEEK_TO_BEGIN);
         sal_uInt32 nType;
@@ -170,8 +150,8 @@ void TypeSerializerTest::testGraphic_Vector()
 
         CPPUNIT_ASSERT_EQUAL(sal_uInt64(233), aMemoryStream.remainingSize());
         std::vector<unsigned char> aHash = calculateHash(aMemoryStream);
-        
CPPUNIT_ASSERT_EQUAL(std::string("c2bed2099ce617f1cc035701de5186f0d43e3064"),
-                             toHexString(aHash));
+        
CPPUNIT_ASSERT_EQUAL(std::string("3dd76dd7533c761eff4805ed28b0d0cc6aa16e58"),
+                             comphelper::hashToString(aHash));
 
         aMemoryStream.Seek(STREAM_SEEK_TO_BEGIN);
         sal_uInt32 nType;
@@ -208,8 +188,8 @@ void TypeSerializerTest::testGraphic_Bitmap_NoGfxLink()
 
         CPPUNIT_ASSERT_EQUAL(sal_uInt64(383), aMemoryStream.remainingSize());
         std::vector<unsigned char> aHash = calculateHash(aMemoryStream);
-        
CPPUNIT_ASSERT_EQUAL(std::string("da831418499146d51bf245fadf60b9111faa76c2"),
-                             toHexString(aHash));
+        
CPPUNIT_ASSERT_EQUAL(std::string("fc008eb6191d74184960cc8f4785d9e08546ee3d"),
+                             comphelper::hashToString(aHash));
 
         aMemoryStream.Seek(STREAM_SEEK_TO_BEGIN);
         sal_uInt16 nType;
@@ -250,8 +230,8 @@ void TypeSerializerTest::testGraphic_Animation()
 
         CPPUNIT_ASSERT_EQUAL(sal_uInt64(15123), aMemoryStream.remainingSize());
         std::vector<unsigned char> aHash = calculateHash(aMemoryStream);
-        
CPPUNIT_ASSERT_EQUAL(std::string("86e02d37ab5e9c96fbeda717f62bc6e35dec3a70"),
-                             toHexString(aHash));
+        
CPPUNIT_ASSERT_EQUAL(std::string("1defb473c6f0eeb915935d5039ce0aaed00226ed"),
+                             comphelper::hashToString(aHash));
 
         aMemoryStream.Seek(STREAM_SEEK_TO_BEGIN);
         sal_uInt16 nType;
@@ -282,8 +262,8 @@ void TypeSerializerTest::testGraphic_Animation()
 
         CPPUNIT_ASSERT_EQUAL(sal_uInt64(1582), aMemoryStream.remainingSize());
         std::vector<unsigned char> aHash = calculateHash(aMemoryStream);
-        
CPPUNIT_ASSERT_EQUAL(std::string("da3b9600340fa80a895f2107357e4ab65a9292eb"),
-                             toHexString(aHash));
+        
CPPUNIT_ASSERT_EQUAL(std::string("60423954a15cc15486162b17d9fb7096d64138d9"),
+                             comphelper::hashToString(aHash));
 
         aMemoryStream.Seek(STREAM_SEEK_TO_BEGIN);
         sal_uInt32 nType;
@@ -328,8 +308,8 @@ void TypeSerializerTest::testGraphic_GDIMetaFile()
 
         CPPUNIT_ASSERT_EQUAL(sal_uInt64(229), aMemoryStream.remainingSize());
         std::vector<unsigned char> aHash = calculateHash(aMemoryStream);
-        
CPPUNIT_ASSERT_EQUAL(std::string("144c518e5149d61ab4bc34643df820372405d61d"),
-                             toHexString(aHash));
+        
CPPUNIT_ASSERT_EQUAL(std::string("607b19f614387aba3fb9dca843962ecb173dca4e"),
+                             comphelper::hashToString(aHash));
 
         aMemoryStream.Seek(STREAM_SEEK_TO_BEGIN);
         char aIdCharArray[7] = { 0, 0, 0, 0, 0, 0, 0 };
diff --git a/vcl/source/graphic/BinaryDataContainer.cxx 
b/vcl/source/graphic/BinaryDataContainer.cxx
index f5ca4a4b309b..3ed9d4020beb 100644
--- a/vcl/source/graphic/BinaryDataContainer.cxx
+++ b/vcl/source/graphic/BinaryDataContainer.cxx
@@ -13,6 +13,7 @@
 #include <unotools/tempfile.hxx>
 #include <comphelper/lok.hxx>
 #include <comphelper/seqstream.hxx>
+#include <comphelper/hash.hxx>
 #include <sal/log.hxx>
 
 #include <vector>
@@ -87,6 +88,13 @@ size_t BinaryDataContainer::calculateHash() const
     return nSeed;
 }
 
+std::vector<unsigned char> BinaryDataContainer::calculateSHA1() const
+{
+    comphelper::Hash aHashEngine(comphelper::HashType::SHA1);
+    aHashEngine.update(getData(), getSize());
+    return aHashEngine.finalize();
+}
+
 css::uno::Sequence<sal_Int8> BinaryDataContainer::getCopyAsByteSequence() const
 {
     if (isEmpty())

Reply via email to