This is an automated email from the ASF dual-hosted git repository.

pengzheng pushed a commit to branch feature/527-manifest-improvement
in repository https://gitbox.apache.org/repos/asf/celix.git


The following commit(s) were added to 
refs/heads/feature/527-manifest-improvement by this push:
     new 78b31d5f Adapt Felix's manifest parsing to Celix.
78b31d5f is described below

commit 78b31d5f8e366d4dff8535c61c5b91964f536911
Author: PengZheng <howto...@gmail.com>
AuthorDate: Wed Aug 16 11:15:50 2023 +0800

    Adapt Felix's manifest parsing to Celix.
    
    Not only does it fix the memory safety issue when dealing with line 
continuation, it is also more memory efficient (0.5K stack usage vs 3.5K stack 
usage).
---
 libs/error_injector/CMakeLists.txt                 |   1 +
 libs/error_injector/stdio/CMakeLists.txt           |   3 +
 libs/error_injector/stdio/include/stdio_ei.h       |   6 +
 libs/error_injector/stdio/src/stdio_ei.cc          |  48 +++-
 .../{stdio => string}/CMakeLists.txt               |  17 +-
 libs/error_injector/string/include/string_ei.h     |  35 +++
 libs/error_injector/string/src/string_ei.cc        |  33 +++
 libs/framework/gtest/CMakeLists.txt                |   2 +
 .../gtest/src/ManifestErrorInjectionTestSuite.cc   |  77 +++++
 libs/framework/gtest/src/ManifestTestSuite.cc      | 113 +++++++-
 libs/framework/src/manifest.c                      | 315 ++++++++++-----------
 libs/utils/gtest/CMakeLists.txt                    |  15 +-
 libs/utils/gtest/src/ErrErrorInjectionTestSuite.cc |   2 -
 .../gtest/src/PropertiesErrorInjectionTestSuite.cc |  38 +++
 libs/utils/src/celix_err.c                         |   4 +-
 15 files changed, 508 insertions(+), 201 deletions(-)

diff --git a/libs/error_injector/CMakeLists.txt 
b/libs/error_injector/CMakeLists.txt
index bd149b46..958d99c9 100644
--- a/libs/error_injector/CMakeLists.txt
+++ b/libs/error_injector/CMakeLists.txt
@@ -27,6 +27,7 @@ add_subdirectory(malloc)
 add_subdirectory(asprintf)
 add_subdirectory(stdio)
 add_subdirectory(stdlib)
+add_subdirectory(string)
 add_subdirectory(eventfd)
 add_subdirectory(stat)
 add_subdirectory(fts)
diff --git a/libs/error_injector/stdio/CMakeLists.txt 
b/libs/error_injector/stdio/CMakeLists.txt
index ae95a525..2068795e 100644
--- a/libs/error_injector/stdio/CMakeLists.txt
+++ b/libs/error_injector/stdio/CMakeLists.txt
@@ -25,5 +25,8 @@ target_link_options(stdio_ei INTERFACE
         LINKER:--wrap,fwrite
         LINKER:--wrap,remove
         LINKER:--wrap,open_memstream
+        LINKER:--wrap,fseek
+        LINKER:--wrap,ftell
+        LINKER:--wrap,fread
         )
 add_library(Celix::stdio_ei ALIAS stdio_ei)
diff --git a/libs/error_injector/stdio/include/stdio_ei.h 
b/libs/error_injector/stdio/include/stdio_ei.h
index 894bed7a..a3018f81 100644
--- a/libs/error_injector/stdio/include/stdio_ei.h
+++ b/libs/error_injector/stdio/include/stdio_ei.h
@@ -34,6 +34,12 @@ CELIX_EI_DECLARE(remove, int);
 
 CELIX_EI_DECLARE(open_memstream, FILE *);
 
+CELIX_EI_DECLARE(fseek, int);
+
+CELIX_EI_DECLARE(ftell, long);
+
+CELIX_EI_DECLARE(fread, size_t);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/libs/error_injector/stdio/src/stdio_ei.cc 
b/libs/error_injector/stdio/src/stdio_ei.cc
index 00555126..ca710752 100644
--- a/libs/error_injector/stdio/src/stdio_ei.cc
+++ b/libs/error_injector/stdio/src/stdio_ei.cc
@@ -21,42 +21,66 @@
 #include "stdio_ei.h"
 
 extern "C" {
-FILE *__real_fopen (const char *__filename, const char *__modes);
-CELIX_EI_DEFINE(fopen, FILE *)
-FILE *__wrap_fopen (const char *__filename, const char *__modes) {
+FILE* __real_fopen(const char* __filename, const char* __modes);
+CELIX_EI_DEFINE(fopen, FILE*)
+FILE* __wrap_fopen(const char* __filename, const char* __modes) {
     errno = EMFILE;
     CELIX_EI_IMPL(fopen);
     errno = 0;
     return __real_fopen(__filename, __modes);
 }
 
-
-size_t __real_fwrite (const void *__restrict __ptr, size_t __size, size_t __n, 
FILE *__restrict __s);
+size_t __real_fwrite(const void* __restrict __ptr, size_t __size, size_t __n, 
FILE* __restrict __s);
 CELIX_EI_DEFINE(fwrite, size_t)
-size_t __wrap_fwrite (const void *__restrict __ptr, size_t __size, size_t __n, 
FILE *__restrict __s) {
+size_t __wrap_fwrite(const void* __restrict __ptr, size_t __size, size_t __n, 
FILE* __restrict __s) {
     errno = ENOSPC;
     CELIX_EI_IMPL(fwrite);
     errno = 0;
     return __real_fwrite(__ptr, __size, __n, __s);
 }
 
-
-int __real_remove (const char *__filename);
+int __real_remove(const char* __filename);
 CELIX_EI_DEFINE(remove, int)
-int __wrap_remove (const char *__filename) {
+int __wrap_remove(const char* __filename) {
     errno = EACCES;
     CELIX_EI_IMPL(remove);
     errno = 0;
     return __real_remove(__filename);
 }
 
-FILE *__real_open_memstream (char **__bufloc, size_t *__sizeloc);
-CELIX_EI_DEFINE(open_memstream, FILE *)
-FILE *__wrap_open_memstream (char **__bufloc, size_t *__sizeloc) {
+FILE* __real_open_memstream(char** __bufloc, size_t* __sizeloc);
+CELIX_EI_DEFINE(open_memstream, FILE*)
+FILE* __wrap_open_memstream(char** __bufloc, size_t* __sizeloc) {
     errno = ENOMEM;
     CELIX_EI_IMPL(open_memstream);
     errno = 0;
     return __real_open_memstream(__bufloc, __sizeloc);
 }
 
+int __real_fseek(FILE* __stream, long int __off, int __whence);
+CELIX_EI_DEFINE(fseek, int)
+int __wrap_fseek(FILE* __stream, long int __off, int __whence) {
+    errno = EACCES;
+    CELIX_EI_IMPL(fseek);
+    errno = 0;
+    return __real_fseek(__stream, __off, __whence);
+}
+
+long __real_ftell(FILE* __stream);
+CELIX_EI_DEFINE(ftell, long)
+long __wrap_ftell(FILE* __stream) {
+    if (ftell_ret == -1) {
+        errno = EACCES;
+    }
+    CELIX_EI_IMPL(ftell);
+    errno = 0;
+    return __real_ftell(__stream);
+}
+
+size_t __real_fread(void* __restrict __ptr, size_t __size, size_t __n, FILE* 
__restrict __s);
+CELIX_EI_DEFINE(fread, size_t)
+size_t __wrap_fread(void* __restrict __ptr, size_t __size, size_t __n, FILE* 
__restrict __s) {
+    CELIX_EI_IMPL(fread);
+    return __real_fread(__ptr, __size, __n, __s);
+}
 }
\ No newline at end of file
diff --git a/libs/error_injector/stdio/CMakeLists.txt 
b/libs/error_injector/string/CMakeLists.txt
similarity index 64%
copy from libs/error_injector/stdio/CMakeLists.txt
copy to libs/error_injector/string/CMakeLists.txt
index ae95a525..ff53b52d 100644
--- a/libs/error_injector/stdio/CMakeLists.txt
+++ b/libs/error_injector/string/CMakeLists.txt
@@ -15,15 +15,12 @@
 # specific language governing permissions and limitations
 # under the License.
 
-add_library(stdio_ei STATIC src/stdio_ei.cc)
+add_library(string_ei STATIC src/string_ei.cc)
 
-target_include_directories(stdio_ei PUBLIC ${CMAKE_CURRENT_LIST_DIR}/include)
-target_link_libraries(stdio_ei PUBLIC Celix::error_injector)
-# It plays nicely with address sanitizer this way.
-target_link_options(stdio_ei INTERFACE
-        LINKER:--wrap,fopen
-        LINKER:--wrap,fwrite
-        LINKER:--wrap,remove
-        LINKER:--wrap,open_memstream
+target_include_directories(string_ei PUBLIC ${CMAKE_CURRENT_LIST_DIR}/include)
+target_link_libraries(string_ei PUBLIC Celix::error_injector)
+
+target_link_options(string_ei INTERFACE
+        LINKER:--wrap,strndup
         )
-add_library(Celix::stdio_ei ALIAS stdio_ei)
+add_library(Celix::string_ei ALIAS string_ei)
diff --git a/libs/error_injector/string/include/string_ei.h 
b/libs/error_injector/string/include/string_ei.h
new file mode 100644
index 00000000..85a2b354
--- /dev/null
+++ b/libs/error_injector/string/include/string_ei.h
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#ifndef CELIX_STRING_EI_H
+#define CELIX_STRING_EI_H
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <string.h>
+#include "celix_error_injector.h"
+
+CELIX_EI_DECLARE(strndup, char *);
+
+#ifdef __cplusplus
+}
+#endif
+#endif // CELIX_STRING_EI_H
diff --git a/libs/error_injector/string/src/string_ei.cc 
b/libs/error_injector/string/src/string_ei.cc
new file mode 100644
index 00000000..74747892
--- /dev/null
+++ b/libs/error_injector/string/src/string_ei.cc
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include "string_ei.h"
+#include <errno.h>
+
+extern "C" {
+char* __real_strndup(const char* s, size_t n);
+CELIX_EI_DEFINE(strndup, char*)
+char* __wrap_strndup(const char* s, size_t n) {
+    errno = ENOMEM;
+    CELIX_EI_IMPL(strndup);
+    errno = 0;
+    return __real_strndup(s, n);
+}
+}
\ No newline at end of file
diff --git a/libs/framework/gtest/CMakeLists.txt 
b/libs/framework/gtest/CMakeLists.txt
index b449defd..19c73f81 100644
--- a/libs/framework/gtest/CMakeLists.txt
+++ b/libs/framework/gtest/CMakeLists.txt
@@ -162,6 +162,8 @@ if (LINKER_WRAP_SUPPORTED)
             Celix::stat_ei
             Celix::threads_ei
             Celix::hmap_ei
+            Celix::stdio_ei
+            Celix::string_ei
             GTest::gtest GTest::gtest_main
     )
 
diff --git a/libs/framework/gtest/src/ManifestErrorInjectionTestSuite.cc 
b/libs/framework/gtest/src/ManifestErrorInjectionTestSuite.cc
index a257bdc6..ae64656b 100644
--- a/libs/framework/gtest/src/ManifestErrorInjectionTestSuite.cc
+++ b/libs/framework/gtest/src/ManifestErrorInjectionTestSuite.cc
@@ -19,14 +19,18 @@
  */
 
 #include <gtest/gtest.h>
+#include <limits.h>
 #include <stdio.h>
 
 #include "celix_err.h"
 #include "celix_properties_ei.h"
 #include "celix_stdio_cleanup.h"
+#include "celix_utils_ei.h"
 #include "hmap_ei.h"
 #include "malloc_ei.h"
 #include "manifest.h"
+#include "stdio_ei.h"
+#include "string_ei.h"
 
 class ManifestErrorInjectionTestSuite : public ::testing::Test {
 public:
@@ -41,6 +45,11 @@ public:
         celix_ei_expect_celix_properties_create(nullptr, 0, nullptr);
         celix_ei_expect_celix_properties_copy(nullptr, 0, nullptr);
         celix_ei_expect_hashMap_create(nullptr, 0, nullptr);
+        celix_ei_expect_celix_utils_strdup(nullptr, 0, nullptr);
+        celix_ei_expect_fseek(nullptr, 0, -1);
+        celix_ei_expect_ftell(nullptr, 0, -1);
+        celix_ei_expect_fread(nullptr, 0, 0);
+        celix_ei_expect_strndup(nullptr, 0, nullptr);
     }
 };
 
@@ -94,6 +103,74 @@ TEST_F(ManifestErrorInjectionTestSuite, 
NoMemoryForManifestCloneTest) {
     EXPECT_EQ(nullptr, manifest_clone(manifest));
     teardownErrorInjectors();
 
+    celix_ei_expect_celix_utils_strdup((void*)manifest_clone, 0, nullptr);
+    EXPECT_EQ(nullptr, manifest_clone(manifest));
+    teardownErrorInjectors();
+
     celix_ei_expect_celix_properties_copy((void*)manifest_clone, 0, nullptr);
     EXPECT_EQ(nullptr, manifest_clone(manifest));
+    teardownErrorInjectors();
+}
+
+TEST_F(ManifestErrorInjectionTestSuite, ReadFromStreamErrorTest) {
+    std::string content = "Manifest-Version: 1.0\n"
+                          "DeploymentPackage-Icon: %icon\n"
+                          "DeploymentPackage-SymbolicName: com.third._3d\n"
+                          "DeploymentPacakge-Version: 1.2.3.build22032005\n"
+                          "\n"
+                          "Name: bundles/3dlib.jar\n"
+                          "SHA1-Digest: MOez1l4gXHBo8ycYdAxstK3UvEg=\n"
+                          "Bundle-SymbolicName: com.third._3d\n"
+                          "Bundle-Version: 2.3.1\n"
+                          "\n"
+                          "Name: bundles/3dnative.jar\n"
+                          "SHA1-Digest: N8Ow2UY4yjnHZv5zeq2I1Uv/+uE=\n"
+                          "Bundle-SymbolicName: com.third._3d.native\n"
+                          "Bundle-Version: 1.5.3\n"
+                          "\n"
+                          "Name: bundles/3dnative1.jar\n"
+                          "SHA1-Digest: N8Ow2UY4yjnHZv5zeq2I1Uv/+uF=\n"
+                          "Bundle-SymbolicName: com.third._3d.native1\n"
+                          "Bundle-Version: 1.5.4\n"
+                          "\n";
+    celix_autoptr(FILE) manifestFile = fmemopen((void*)content.c_str(), 
content.size(), "r");
+    celix_auto(manifest_pt) manifest = nullptr;
+    manifest_create(&manifest);
+    celix_ei_expect_fseek((void*)manifest_readFromStream, 0, -1);
+    EXPECT_EQ(EACCES, manifest_readFromStream(manifest, manifestFile));
+    teardownErrorInjectors();
+
+    celix_ei_expect_ftell((void*)manifest_readFromStream, 0, -1);
+    EXPECT_EQ(EACCES, manifest_readFromStream(manifest, manifestFile));
+    teardownErrorInjectors();
+
+    celix_ei_expect_ftell((void*)manifest_readFromStream, 0, (long)INT_MAX+1);
+    EXPECT_EQ(CELIX_BUNDLE_EXCEPTION, manifest_readFromStream(manifest, 
manifestFile));
+    celix_err_printErrors(stdout, "Errors are expected[", "]\n");
+    teardownErrorInjectors();
+
+    celix_ei_expect_malloc((void*)manifest_readFromStream, 0, nullptr);
+    EXPECT_EQ(CELIX_ENOMEM, manifest_readFromStream(manifest, manifestFile));
+    celix_err_printErrors(stdout, "Errors are expected[", "]\n");
+    teardownErrorInjectors();
+
+    celix_ei_expect_fread((void*)manifest_readFromStream, 0, 1);
+    EXPECT_EQ(CELIX_FILE_IO_EXCEPTION, manifest_readFromStream(manifest, 
manifestFile));
+    teardownErrorInjectors();
+
+    for (int i = 0; i < 3; ++i) {
+        celix_auto(manifest_pt) mfs = nullptr;
+        manifest_create(&mfs);
+        celix_ei_expect_celix_utils_strdup((void*)manifest_readFromStream, 0, 
nullptr, i+1);
+        EXPECT_EQ(CELIX_ENOMEM, manifest_readFromStream(mfs, manifestFile));
+        teardownErrorInjectors();
+    }
+
+    for (int i = 0; i < 3; ++i) {
+        celix_auto(manifest_pt) mfs = nullptr;
+        manifest_create(&mfs);
+        
celix_ei_expect_celix_properties_create((void*)manifest_readFromStream, 0, 
nullptr, i+1);
+        EXPECT_EQ(CELIX_ENOMEM, manifest_readFromStream(mfs, manifestFile));
+        teardownErrorInjectors();
+    }
 }
\ No newline at end of file
diff --git a/libs/framework/gtest/src/ManifestTestSuite.cc 
b/libs/framework/gtest/src/ManifestTestSuite.cc
index 2b806137..bcbfce3a 100644
--- a/libs/framework/gtest/src/ManifestTestSuite.cc
+++ b/libs/framework/gtest/src/ManifestTestSuite.cc
@@ -26,6 +26,7 @@
 #include "fmemopen.h"
 #endif
 
+#include "celix_err.h"
 #include "celix_stdio_cleanup.h"
 #include "manifest.h"
 
@@ -37,6 +38,7 @@ protected:
     }
     void TearDown() override {
         manifest_destroy(manifest);
+        celix_err_resetErrors();
     }
     void CheckPropertiesEqual(const celix_properties_t* prop1, const 
celix_properties_t* prop2) {
         EXPECT_EQ(celix_properties_size(prop1), celix_properties_size(prop2));
@@ -90,6 +92,25 @@ TEST_F(ManifestTestSuite, 
ReadManifestWithoutNameSectionsTest) {
     EXPECT_EQ(0, hashMap_size(entries));
 }
 
+TEST_F(ManifestTestSuite, ReadManifestWithoutNewlineInLastLineTest) {
+    std::string content = "Manifest-Version: 1.0\n"
+                          "DeploymentPackage-SymbolicName: com.third._3d\n"
+                          "DeploymentPacakge-Version: 1.2.3.build22032005\n"
+                          "DeploymentPackage-Copyright: ACME Inc. (c) 2003";
+    celix_autoptr(FILE) manifestFile = fmemopen((void*)content.c_str(), 
content.size(), "r");
+    EXPECT_EQ(CELIX_SUCCESS, manifest_readFromStream(manifest, manifestFile));
+    const celix_properties_t* mainAttr = manifest_getMainAttributes(manifest);
+    EXPECT_EQ(4, celix_properties_size(mainAttr));
+    EXPECT_STREQ("1.0", celix_properties_get(mainAttr, "Manifest-Version", 
nullptr));
+    EXPECT_STREQ("com.third._3d", celix_properties_get(mainAttr, 
"DeploymentPackage-SymbolicName", nullptr));
+    EXPECT_STREQ("1.2.3.build22032005", celix_properties_get(mainAttr, 
"DeploymentPacakge-Version", nullptr));
+    EXPECT_STREQ("ACME Inc. (c) 2003", celix_properties_get(mainAttr, 
"DeploymentPackage-Copyright", nullptr));
+    hash_map_pt entries = nullptr;
+    EXPECT_EQ(CELIX_SUCCESS, manifest_getEntries(manifest, &entries));
+    EXPECT_NE(nullptr, entries);
+    EXPECT_EQ(0, hashMap_size(entries));
+}
+
 TEST_F(ManifestTestSuite, ReadManifestWithNameSectionsTest) {
     std::string content = "Manifest-Version: 1.0\n"
                           "DeploymentPackage-Icon: %icon\n"
@@ -105,6 +126,11 @@ TEST_F(ManifestTestSuite, 
ReadManifestWithNameSectionsTest) {
                           "SHA1-Digest: N8Ow2UY4yjnHZv5zeq2I1Uv/+uE=\n"
                           "Bundle-SymbolicName: com.third._3d.native\n"
                           "Bundle-Version: 1.5.3\n"
+                          "\n"
+                          "Name: bundles/3dnative1.jar\n"
+                          "SHA1-Digest: N8Ow2UY4yjnHZv5zeq2I1Uv/+uF=\n"
+                          "Bundle-SymbolicName: com.third._3d.native1\n"
+                          "Bundle-Version: 1.5.4\n"
                           "\n";
     celix_autoptr(FILE) manifestFile = fmemopen((void*)content.c_str(), 
content.size(), "r");
     EXPECT_EQ(CELIX_SUCCESS, manifest_readFromStream(manifest, manifestFile));
@@ -117,16 +143,16 @@ TEST_F(ManifestTestSuite, 
ReadManifestWithNameSectionsTest) {
     hash_map_pt entries = nullptr;
     EXPECT_EQ(CELIX_SUCCESS, manifest_getEntries(manifest, &entries));
     EXPECT_NE(nullptr, entries);
-    EXPECT_EQ(2, hashMap_size(entries));
+    EXPECT_EQ(3, hashMap_size(entries));
 
     const celix_properties_t* entry1 = (const 
celix_properties_t*)hashMap_get(entries, "bundles/3dlib.jar");
-    EXPECT_EQ(3, celix_properties_size(entry1));
+    EXPECT_EQ(4, celix_properties_size(entry1));
     EXPECT_STREQ("MOez1l4gXHBo8ycYdAxstK3UvEg=", celix_properties_get(entry1, 
"SHA1-Digest", nullptr));
     EXPECT_STREQ("com.third._3d", celix_properties_get(entry1, 
"Bundle-SymbolicName", nullptr));
     EXPECT_STREQ("2.3.1", celix_properties_get(entry1, "Bundle-Version", 
nullptr));
 
     const celix_properties_t* entry2 = (const 
celix_properties_t*)hashMap_get(entries, "bundles/3dnative.jar");
-    EXPECT_EQ(3, celix_properties_size(entry2));
+    EXPECT_EQ(4, celix_properties_size(entry2));
     EXPECT_STREQ("N8Ow2UY4yjnHZv5zeq2I1Uv/+uE=", celix_properties_get(entry2, 
"SHA1-Digest", nullptr));
     EXPECT_STREQ("com.third._3d.native", celix_properties_get(entry2, 
"Bundle-SymbolicName", nullptr));
     EXPECT_STREQ("1.5.3", celix_properties_get(entry2, "Bundle-Version", 
nullptr));
@@ -160,4 +186,83 @@ TEST_F(ManifestTestSuite, CloneTest) {
     EXPECT_EQ(CELIX_SUCCESS, manifest_readFromStream(manifest, manifestFile));
     celix_auto(manifest_pt) clone = manifest_clone(manifest);
     CheckManifestEqual(manifest, clone);
-}
\ No newline at end of file
+}
+
+TEST_F(ManifestTestSuite, CreateFromNonexistingFile) {
+    celix_auto(manifest_pt) manifest2 = nullptr;
+    EXPECT_EQ(ENOENT, manifest_createFromFile("nonexisting", &manifest2));
+    EXPECT_EQ(nullptr, manifest2);
+    celix_err_printErrors(stdout, "Errors are expected[", "]\n");
+}
+
+TEST_F(ManifestTestSuite, ManifestMissingSpaceSeparatorTest) {
+    std::string content = "Manifest-Version: 1.0\n"
+                          "NoSeparator:%icon\n"
+                          "DeploymentPackage-SymbolicName: com.third._3d\n"
+                          "DeploymentPacakge-Version: 1.2.3.build22032005\n"
+                          "\n";
+    celix_autoptr(FILE) manifestFile = fmemopen((void*)content.c_str(), 
content.size(), "r");
+    EXPECT_EQ(CELIX_INVALID_SYNTAX, manifest_readFromStream(manifest, 
manifestFile));
+    celix_err_printErrors(stdout, "Errors are expected[", "]\n");
+}
+
+TEST_F(ManifestTestSuite, ManifestMissingAttributeNameTest) {
+    std::string content = "Manifest-Version: 1.0\n"
+                          "%icon\n"
+                          "DeploymentPackage-SymbolicName: com.third._3d\n"
+                          "DeploymentPacakge-Version: 1.2.3.build22032005\n"
+                          "\n";
+    celix_autoptr(FILE) manifestFile = fmemopen((void*)content.c_str(), 
content.size(), "r");
+    EXPECT_EQ(CELIX_INVALID_SYNTAX, manifest_readFromStream(manifest, 
manifestFile));
+    celix_err_printErrors(stdout, "Errors are expected[", "]\n");
+}
+
+TEST_F(ManifestTestSuite, ManifestWithDuplicatedAttributeNameTest) {
+    std::string content = "Manifest-Version: 1.0\n"
+                          "DeploymentPackage-Icon: %icon\n"
+                          "DeploymentPackage-Icon: %icon\n"
+                          "DeploymentPackage-SymbolicName: com.third._3d\n"
+                          "DeploymentPacakge-Version: 1.2.3.build22032005\n"
+                          "\n";
+    celix_autoptr(FILE) manifestFile = fmemopen((void*)content.c_str(), 
content.size(), "r");
+    EXPECT_EQ(CELIX_INVALID_SYNTAX, manifest_readFromStream(manifest, 
manifestFile));
+    celix_err_printErrors(stdout, "Errors are expected[", "]\n");
+}
+
+TEST_F(ManifestTestSuite, ManifestMissingNameAttributeTest) {
+    std::string content = "Manifest-Version: 1.0\n"
+                          "DeploymentPackage-Icon: %icon\n"
+                          "DeploymentPackage-SymbolicName: com.third._3d\n"
+                          "DeploymentPacakge-Version: 1.2.3.build22032005\n"
+                          "\n"
+                          "SHA1-Digest: MOez1l4gXHBo8ycYdAxstK3UvEg=\n"
+                          "Bundle-SymbolicName: com.third._3d\n"
+                          "Bundle-Version: 2.3.1\n"
+                          "\n"
+                          "Name: bundles/3dnative.jar\n"
+                          "SHA1-Digest: N8Ow2UY4yjnHZv5zeq2I1Uv/+uE=\n"
+                          "Bundle-SymbolicName: com.third._3d.native\n"
+                          "Bundle-Version: 1.5.3\n"
+                          "\n";
+    celix_autoptr(FILE) manifestFile = fmemopen((void*)content.c_str(), 
content.size(), "r");
+    EXPECT_EQ(CELIX_INVALID_SYNTAX, manifest_readFromStream(manifest, 
manifestFile));
+    celix_err_printErrors(stdout, "Errors are expected[", "]\n");
+}
+
+TEST_F(ManifestTestSuite, ManifestMissingVersionTest) {
+    std::string content = /*"Manifest-Version: 1.0\n"*/
+                          "DeploymentPackage-Icon: %icon\n"
+                          "DeploymentPackage-SymbolicName: com.third._3d\n"
+                          "DeploymentPacakge-Version: 1.2.3.build22032005\n"
+                          "\n";
+    celix_autoptr(FILE) manifestFile = fmemopen((void*)content.c_str(), 
content.size(), "r");
+    EXPECT_EQ(CELIX_INVALID_SYNTAX, manifest_readFromStream(manifest, 
manifestFile));
+    celix_err_printErrors(stdout, "Errors are expected[", "]\n");
+}
+
+TEST_F(ManifestTestSuite, ReadEmptyManifestTest) {
+    std::string content = "\n";
+    celix_autoptr(FILE) manifestFile = fmemopen((void*)content.c_str(), 
content.size(), "r");
+    EXPECT_EQ(CELIX_INVALID_SYNTAX, manifest_readFromStream(manifest, 
manifestFile));
+    celix_err_printErrors(stdout, "Errors are expected[", "]\n");
+}
diff --git a/libs/framework/src/manifest.c b/libs/framework/src/manifest.c
index 5bf231d8..06936dbb 100644
--- a/libs/framework/src/manifest.c
+++ b/libs/framework/src/manifest.c
@@ -24,6 +24,7 @@
  *  \copyright Apache License, Version 2.0
  */
 
+#include <limits.h>
 #include <stdlib.h>
 #include <string.h>
 #include <stdbool.h>
@@ -32,13 +33,10 @@
 #include "celix_errno.h"
 #include "celix_stdio_cleanup.h"
 #include "celix_stdlib_cleanup.h"
+#include "celix_utils.h"
 #include "manifest.h"
 #include "utils.h"
 
-int fpeek(FILE *stream);
-
-static celix_status_t manifest_readAttributes(manifest_pt manifest, 
properties_pt properties, FILE *file);
-
 celix_status_t manifest_create(manifest_pt *manifest) {
     celix_status_t status = CELIX_SUCCESS;
     do {
@@ -81,12 +79,16 @@ manifest_pt manifest_clone(manifest_pt manifest) {
         hash_map_iterator_t iter = 
hashMapIterator_construct(manifest->attributes);
         while (hashMapIterator_hasNext(&iter)) {
             hash_map_entry_pt entry = hashMapIterator_nextEntry(&iter);
+            celix_autofree char* attrKey = 
celix_utils_strdup(hashMapEntry_getKey(entry));
+            if (attrKey == NULL) {
+                return NULL;
+            }
             celix_properties_t* value = hashMapEntry_getValue(entry);
             celix_properties_t* cloneValue = celix_properties_copy(value);
             if (cloneValue == NULL) {
                 return NULL;
             }
-            hashMap_put(clone->attributes, strdup(hashMapEntry_getKey(entry)), 
cloneValue);
+            hashMap_put(clone->attributes, celix_steal_ptr(attrKey), 
cloneValue);
         }
     }
 
@@ -114,16 +116,15 @@ celix_status_t manifest_destroy(manifest_pt manifest) {
 celix_status_t manifest_createFromFile(const char *filename, manifest_pt 
*manifest) {
     celix_status_t status;
 
-    status = manifest_create(manifest);
+    celix_auto(manifest_pt) manifestNew = NULL;
+    status = manifest_create(&manifestNew);
 
+    status = CELIX_DO_IF(status, manifest_read(manifestNew, filename));
     if (status == CELIX_SUCCESS) {
-        manifest_read(*manifest, filename);
-    }
-
-    if (status != CELIX_SUCCESS) {
+        *manifest = celix_steal_ptr(manifestNew);
+    } else {
         celix_err_pushf("Cannot create manifest from file: %s", 
celix_strerror(status));
     }
-
     return status;
 }
 
@@ -132,12 +133,12 @@ void manifest_clear(manifest_pt manifest) {
 }
 
 properties_pt manifest_getMainAttributes(manifest_pt manifest) {
-       return manifest->mainAttributes;
+    return manifest->mainAttributes;
 }
 
 celix_status_t manifest_getEntries(manifest_pt manifest, hash_map_pt *map) {
-       *map = manifest->attributes;
-       return CELIX_SUCCESS;
+    *map = manifest->attributes;
+    return CELIX_SUCCESS;
 }
 
 celix_status_t manifest_read(manifest_pt manifest, const char *filename) {
@@ -147,7 +148,7 @@ celix_status_t manifest_read(manifest_pt manifest, const 
char *filename) {
     if (file != NULL) {
         status = manifest_readFromStream(manifest, file);
     } else {
-        status = CELIX_FILE_IO_EXCEPTION;
+        status = CELIX_ERROR_MAKE(CELIX_FACILITY_CERRNO,errno);
     }
 
     if (status != CELIX_SUCCESS) {
@@ -158,178 +159,154 @@ celix_status_t manifest_read(manifest_pt manifest, 
const char *filename) {
 }
 
 celix_status_t manifest_readFromStream(manifest_pt manifest, FILE* stream) {
-    celix_status_t status = CELIX_SUCCESS;
-
     char lbuf[512];
-    char name[512];
-    bool skipEmptyLines = true;
-    char lastline[512];
-    memset(lbuf,0,512);
-    memset(name,0,512);
-    memset(lastline,0,512);
-    const char* reason = NULL;
-
-    manifest_readAttributes(manifest, manifest->mainAttributes, stream);
-
-    while (status==CELIX_SUCCESS && fgets(lbuf, sizeof(lbuf), stream) != NULL) 
{
-        properties_pt attributes;
-        int len = strlen(lbuf);
+    char *bytes = lbuf;
 
-        if (lbuf[--len] != '\n') {
-            status = CELIX_FILE_IO_EXCEPTION;
-            reason = "Manifest line too long";
-            break;
-        }
-        if (len > 0 && lbuf[len - 1] == '\r') {
-            --len;
-        }
-        if (len == 0 && skipEmptyLines) {
-            continue;
+    // get file size
+    if(fseek(stream, 0L, SEEK_END) == -1) {
+        return CELIX_ERROR_MAKE(CELIX_FACILITY_CERRNO,errno);
+    }
+    long int size = ftell(stream);
+    if (size < 0) {
+        return CELIX_ERROR_MAKE(CELIX_FACILITY_CERRNO,errno);
+    } else if (size >= INT_MAX) {
+        celix_err_pushf("Manifest error: file too large - %ld", size);
+        return CELIX_BUNDLE_EXCEPTION;
+    }
+    rewind(stream);
+
+    celix_autofree char* largeBuf = NULL;
+    if (size+1 > sizeof(lbuf)) {
+        largeBuf = bytes =  malloc(size+1);
+        if (largeBuf == NULL) {
+            celix_err_pushf("Manifest error: failed to allocate %ld bytes", 
size);
+            return CELIX_ENOMEM;
         }
-        skipEmptyLines = false;
-
-        if (strlen(name) == 0) {
+    }
 
-            if ((tolower(lbuf[0]) == 'n') && (tolower(lbuf[1]) == 'a') &&
-                (tolower(lbuf[2]) == 'm') && (tolower(lbuf[3]) == 'e') &&
-                (lbuf[4] == ':') && (lbuf[5] == ' ')) {
-                name[0] = '\0';
-                strncpy(name, lbuf+6, len - 6);
-                name[len - 6] = '\0';
-            } else {
-                status = CELIX_FILE_IO_EXCEPTION;
-                reason = "Manifest invalid format";
-                break;
+    if(fread(bytes, 1, size, stream) != (size_t)size) {
+        return CELIX_FILE_IO_EXCEPTION;
+    }
+    // Force a new line at the end of the manifest to deal with broken 
manifest without any line-ending
+    bytes[size++] = '\n';
+
+    const char* key = NULL;
+    int last = 0;
+    int current = 0;
+    celix_properties_t *headers = manifest->mainAttributes;
+    for (int i = 0; i < size; i++)
+    {
+        // skip \r and \n if it is followed by another \n
+        // (we catch the blank line case in the next iteration)
+        if (bytes[i] == '\r')
+        {
+            if ((i + 1 < size) && (bytes[i + 1] == '\n'))
+            {
+                continue;
             }
-
-            if (fpeek(stream) == ' ') {
-                int newlen = len - 6;
-                lastline[0] = '\0';
-                strncpy(lastline, lbuf+6, len - 6);
-                lastline[newlen] = '\0';
+        }
+        if (bytes[i] == '\n')
+        {
+            if ((i + 1 < size) && (bytes[i + 1] == ' '))
+            {
+                i++;
                 continue;
             }
-        } else {
-            int newlen = strlen(lastline) + len;
-            char buf[512];
-            buf[0] = '\0';
-            strcpy(buf, lastline);
-            strncat(buf, lbuf+1, len - 1);
-            buf[newlen] = '\0';
-
-            if (fpeek(stream) == ' ') {
-                lastline[0] = '\0';
-                strcpy(lastline, buf);
+        }
+        // If we don't have a key yet and see the first : we parse it as the 
key
+        // and skip the :<blank> that follows it.
+        if ((key == NULL) && (bytes[i] == ':'))
+        {
+            key = &bytes[last];
+            // precondition: current <= i
+            bytes[current++] = '\0';
+            if ((i + 1 < size) && (bytes[i + 1] == ' '))
+            {
+                last = current + 1;
                 continue;
             }
-            name[0] = '\0';
-            strcpy(name, buf);
-            name[strlen(buf)] = '\0';
+            else
+            {
+                celix_err_pushf("Manifest error: Missing space separator - 
%s", key);
+                return CELIX_INVALID_SYNTAX;
+            }
         }
-
-        attributes = hashMap_get(manifest->attributes, name);
-        if (attributes == NULL) {
-            attributes = properties_create();
-            hashMap_put(manifest->attributes, strdup(name), attributes);
+        // if we are at the end of a line
+        if (bytes[i] == '\n')
+        {
+            // and it is a blank line stop parsing (main attributes are done)
+            if ((last == current) && (key == NULL))
+            {
+                headers = NULL;
+                continue;
+            }
+            // Otherwise, parse the value and add it to the map (we return
+            // if we don't have a key or the key already exist).
+            const char* value = &bytes[last];
+            // precondition: current <= i
+            bytes[current++] = '\0';
+
+            if (key == NULL)
+            {
+                celix_err_pushf("Manifest error: Missing attribute name - %s", 
value);
+                return CELIX_INVALID_SYNTAX;
+            }
+            else if (headers != NULL && celix_properties_get(headers, key, 
NULL) != NULL)
+            {
+                celix_err_pushf("Manifest error: Duplicate attribute name - 
%s", key);
+                return CELIX_INVALID_SYNTAX;
+            }
+            if (headers == NULL) {
+                // beginning of individual-section
+                if (strcasecmp(key, "Name")) {
+                    celix_err_push("Manifest error: individual-section missing 
Name attribute");
+                    return CELIX_INVALID_SYNTAX;
+                }
+                headers = hashMap_get(manifest->attributes, value);
+                if (headers == NULL) {
+                    celix_autofree char* name = celix_utils_strdup(value);
+                    if (name == NULL) {
+                        return CELIX_ENOMEM;
+                    }
+                    headers = celix_properties_create();
+                    if (headers == NULL) {
+                        return CELIX_ENOMEM;
+                    }
+                    hashMap_put(manifest->attributes, 
(void*)celix_steal_ptr(name), headers);
+                }
+            } else if (headers == manifest->mainAttributes && 
celix_properties_size(headers) == 0) {
+                // beginning of main-section
+                if (strcasecmp(key, "Manifest-Version")) {
+                    celix_err_push("Manifest error: main-section must start 
with Manifest-Version");
+                    return CELIX_INVALID_SYNTAX;
+                }
+            }
+            celix_properties_set(headers, key, value);
+            last = current;
+            key = NULL;
+        }
+        else
+        {
+            // precondition: current <= i
+            // write back the byte if it needs to be included in the key or 
the value.
+            bytes[current++] = bytes[i];
         }
-        manifest_readAttributes(manifest, attributes, stream);
-
-        name[0] = '\0';
-        skipEmptyLines = true;
     }
-
-    if (status != CELIX_SUCCESS) {
-        celix_err_pushf("Cannot read manifest for reason %s: %s", reason, 
celix_strerror(status));
+    if (celix_properties_size(manifest->mainAttributes) == 0) {
+        celix_err_push("Manifest error: Empty");
+        return CELIX_INVALID_SYNTAX;
     }
-
-    return status;
+    return CELIX_SUCCESS;
 }
 
+//LCOV_EXCL_START
 void manifest_write(manifest_pt manifest, const char * filename) {
 
 }
+//LCOV_EXCL_STOP
 
 const char* manifest_getValue(manifest_pt manifest, const char* name) {
-       const char* val = properties_get(manifest->mainAttributes, name);
-       bool isEmpty = utils_isStringEmptyOrNull(val);
-       return isEmpty ? NULL : val;
-}
-
-int fpeek(FILE *stream) {
-       int c;
-       c = fgetc(stream);
-       ungetc(c, stream);
-       return c;
-}
-
-static celix_status_t manifest_readAttributes(manifest_pt manifest, 
properties_pt properties, FILE *file) {
-    char name[512]; memset(name,0,512);
-    char value[512]; memset(value,0,512);
-    char lastLine[512]; memset(lastLine,0,512);
-    char lbuf[512]; memset(lbuf,0,512);
-
-
-    while (fgets(lbuf, sizeof(lbuf), file ) != NULL ) {
-        int len = strlen(lbuf);
-
-        if (lbuf[--len] != '\n') {
-            celix_err_push("MANIFEST: Line too long");
-            return CELIX_FILE_IO_EXCEPTION;
-        }
-        if (len > 0 && lbuf[len - 1] == '\r') {
-            --len;
-        }
-        if (len == 0) {
-            break;
-        }
-
-        if (lbuf[0] == ' ') {
-            char buf[512];
-            buf[0] = '\0';
-
-            // Line continued
-            strcat(buf, lastLine);
-            strncat(buf, lbuf+1, len - 1);
-
-            if (fpeek(file) == ' ') {
-//              lastLine = realloc(lastLine, strlen(buf) + 1);
-                lastLine[0] = '\0';
-                strcpy(lastLine, buf);
-                continue;
-            }
-            value[0] = '\0';
-            strcpy(value, buf);
-        } else {
-            int i = 0;
-            while (lbuf[i++] != ':') {
-                if (i >= len) {
-
-                    celix_err_push("MANIFEST: Invalid header");
-                    return CELIX_FILE_IO_EXCEPTION;
-                }
-            }
-            if (lbuf[i++] != ' ') {
-                celix_err_push("MANIFEST: Invalid header");
-                return CELIX_FILE_IO_EXCEPTION;
-            }
-            name[0] = '\0';
-            strncpy(name, lbuf, i - 2);
-            name[i - 2] = '\0';
-            if (fpeek(file) == ' ') {
-                int newlen = len - i;
-                lastLine[0] = '\0';
-                strncpy(lastLine, lbuf+i, len -i);
-                lastLine[newlen] = '\0';
-                continue;
-            }
-            value[0] = '\0';
-            strncpy(value, lbuf+i, len - i);
-            value[len - i] = '\0';
-        }
-
-        properties_set(properties, name, value);
-    }
-
-    return CELIX_SUCCESS;
+    const char* val = properties_get(manifest->mainAttributes, name);
+    bool isEmpty = utils_isStringEmptyOrNull(val);
+    return isEmpty ? NULL : val;
 }
-
diff --git a/libs/utils/gtest/CMakeLists.txt b/libs/utils/gtest/CMakeLists.txt
index f4c776cc..a378aa57 100644
--- a/libs/utils/gtest/CMakeLists.txt
+++ b/libs/utils/gtest/CMakeLists.txt
@@ -89,8 +89,21 @@ if (LINKER_WRAP_SUPPORTED)
             src/IpUtilsErrorInjectionTestSuite.cc
             src/ArrayListErrorInjectionTestSuite.cc
             src/ErrErrorInjectionTestSuite.cc
+            src/PropertiesErrorInjectionTestSuite.cc
+    )
+    target_link_libraries(test_utils_with_ei PRIVATE
+            Celix::zip_ei
+            Celix::stdio_ei
+            Celix::stat_ei
+            Celix::fts_ei
+            utils_cut
+            Celix::utils_ei
+            Celix::ifaddrs_ei
+            Celix::threads_ei
+            Celix::malloc_ei
+            Celix::hmap_ei
+            GTest::gtest GTest::gtest_main
     )
-    target_link_libraries(test_utils_with_ei PRIVATE Celix::zip_ei 
Celix::stdio_ei Celix::stat_ei Celix::fts_ei utils_cut Celix::utils_ei 
Celix::ifaddrs_ei Celix::threads_ei Celix::malloc_ei GTest::gtest 
GTest::gtest_main)
     target_include_directories(test_utils_with_ei PRIVATE ../src) #for 
version_private (needs refactoring of test)
     celix_deprecated_utils_headers(test_utils_with_ei)
     add_dependencies(test_utils_with_ei test_utils_resources)
diff --git a/libs/utils/gtest/src/ErrErrorInjectionTestSuite.cc 
b/libs/utils/gtest/src/ErrErrorInjectionTestSuite.cc
index 402199b4..1fc88fd7 100644
--- a/libs/utils/gtest/src/ErrErrorInjectionTestSuite.cc
+++ b/libs/utils/gtest/src/ErrErrorInjectionTestSuite.cc
@@ -39,7 +39,6 @@ public:
     }
 };
 
-#ifndef CELIX_ERR_USE_THREAD_LOCAL
 TEST_F(ErrErrorInjectionTestSuite, PushErrorWithTssSetFailingTest) {
     //Given a primed error injection for celix_tss_set
     celix_ei_expect_celix_tss_set((void*)celix_err_push, 1, 
CELIX_ILLEGAL_STATE);
@@ -73,4 +72,3 @@ TEST_F(ErrErrorInjectionTestSuite, 
PushErrorWithMallocFailingTest) {
     EXPECT_TRUE(strstr(fileContents.c_str(), "Failed to allocate memory for 
celix_err") != nullptr) <<
         "Expected error message not found in: " << fileContents;
 }
-#endif
diff --git a/libs/utils/gtest/src/PropertiesErrorInjectionTestSuite.cc 
b/libs/utils/gtest/src/PropertiesErrorInjectionTestSuite.cc
new file mode 100644
index 00000000..13f1f6b6
--- /dev/null
+++ b/libs/utils/gtest/src/PropertiesErrorInjectionTestSuite.cc
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+#include "celix_properties.h"
+#include "hmap_ei.h"
+
+#include <gtest/gtest.h>
+
+class PropertiesErrorInjectionTestSuite : public ::testing::Test {
+public:
+    PropertiesErrorInjectionTestSuite() = default;
+    ~PropertiesErrorInjectionTestSuite() override {
+        celix_ei_expect_hashMap_create(nullptr, 0, nullptr);
+    }
+};
+
+TEST_F(PropertiesErrorInjectionTestSuite, CopyFailureTest) {
+    celix_autoptr(celix_properties_t) prop = celix_properties_create();
+    ASSERT_NE(nullptr, prop);
+    celix_ei_expect_hashMap_create((void *) &celix_properties_create, 0, 
nullptr);
+    ASSERT_EQ(nullptr, celix_properties_copy(prop));
+}
\ No newline at end of file
diff --git a/libs/utils/src/celix_err.c b/libs/utils/src/celix_err.c
index edc76329..747dcf3f 100644
--- a/libs/utils/src/celix_err.c
+++ b/libs/utils/src/celix_err.c
@@ -22,11 +22,9 @@
 #include <stdarg.h>
 #include <stdio.h>
 #include <string.h>
-
-#ifndef CELIX_ERR_USE_THREAD_LOCAL
 #include <stdlib.h>
+
 #include "celix_threads.h"
-#endif
 
 typedef struct celix_err {
     char buffer[CELIX_ERR_BUFFER_SIZE];


Reply via email to