You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by to...@apache.org on 2016/09/27 15:06:18 UTC

avro git commit: AVRO-1853: Compiler::toBin crashes on empty string. Contributed by Zoltan Ivanfi.

Repository: avro
Updated Branches:
  refs/heads/master 1296ce923 -> bc3fffb06


AVRO-1853: Compiler::toBin crashes on empty string. Contributed by Zoltan Ivanfi.

Added a unit test triggering the bug then fixed the bug. Modified build
process to use debug builds for tests to allow catching bugs of this
type.


Project: http://git-wip-us.apache.org/repos/asf/avro/repo
Commit: http://git-wip-us.apache.org/repos/asf/avro/commit/bc3fffb0
Tree: http://git-wip-us.apache.org/repos/asf/avro/tree/bc3fffb0
Diff: http://git-wip-us.apache.org/repos/asf/avro/diff/bc3fffb0

Branch: refs/heads/master
Commit: bc3fffb067c93ebece18a80f455ea4662a240e0b
Parents: 1296ce9
Author: Zoltan Ivanfi <zi...@cloudera.com>
Authored: Tue Sep 20 19:11:18 2016 +0200
Committer: Tom White <to...@cloudera.com>
Committed: Tue Sep 27 16:05:15 2016 +0100

----------------------------------------------------------------------
 CHANGES.txt                    |  3 ++
 lang/c++/CMakeLists.txt        |  6 ++++
 lang/c++/build.sh              |  8 +++--
 lang/c++/impl/Compiler.cc      |  7 ++--
 lang/c++/test/CompilerTests.cc | 70 +++++++++++++++++++++++++++++++++++++
 5 files changed, 88 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/avro/blob/bc3fffb0/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index d9b72ed..79c655c 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -68,6 +68,9 @@ Trunk (not yet released)
     AVRO-1849: C++: printJson fails on record with no fields.
     (Simon Woodford vai tomwhite)
 
+    AVRO-1853: C++: Compiler::toBin crashes on empty string.
+    (Zoltan Ivanfi via tomwhite)
+
 Avro 1.8.1 (14 May 2016)
 
   INCOMPATIBLE CHANGES

http://git-wip-us.apache.org/repos/asf/avro/blob/bc3fffb0/lang/c++/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/lang/c++/CMakeLists.txt b/lang/c++/CMakeLists.txt
index fc54a71..9927ca7 100644
--- a/lang/c++/CMakeLists.txt
+++ b/lang/c++/CMakeLists.txt
@@ -49,6 +49,11 @@ endif()
 
 if (CMAKE_COMPILER_IS_GNUCXX)
     set(CMAKE_CXX_FLAGS "-Wall")
+    set(CMAKE_CXX_FLAGS_DEBUG "-fstack-protector-all -D_GLIBCXX_DEBUG")
+    # Unset _GLIBCXX_DEBUG for avrogencpp.cc because using Boost Program Options
+    # leads to linking errors when compiling with _GLIBCXX_DEBUG as described on
+    # http://stackoverflow.com/questions/19729036/
+    set_source_files_properties(impl/avrogencpp.cc PROPERTIES COMPILE_FLAGS "-U_GLIBCXX_DEBUG")
 endif ()
 
 
@@ -142,6 +147,7 @@ unittest (SpecificTests)
 unittest (DataFileTests)
 unittest (JsonTests)
 unittest (AvrogencppTests)
+unittest (CompilerTests)
 
 add_dependencies (AvrogencppTests bigrecord_hh bigrecord_r_hh bigrecord2_hh
     tweet_hh

http://git-wip-us.apache.org/repos/asf/avro/blob/bc3fffb0/lang/c++/build.sh
----------------------------------------------------------------------
diff --git a/lang/c++/build.sh b/lang/c++/build.sh
index c8281b4..17fff40 100755
--- a/lang/c++/build.sh
+++ b/lang/c++/build.sh
@@ -18,7 +18,7 @@
 set -e # exit on error
 
 function usage {
-  echo "Usage: $0 {test|dist|clean}"
+  echo "Usage: $0 {test|dist|clean|install|doc}"
   exit 1
 }
 
@@ -76,10 +76,11 @@ function do_dist() {
 
 case "$target" in
   test)
-    (cd build && make && cd .. \
+    (cd build && cmake -G "Unix Makefiles" -D CMAKE_BUILD_TYPE=Debug .. && make && cd .. \
       && ./build/buffertest \
       && ./build/unittest \
       && ./build/CodecTests \
+      && ./build/CompilerTests \
       && ./build/StreamTests \
       && ./build/SpecificTests \
       && ./build/AvrogencppTests \
@@ -88,6 +89,7 @@ case "$target" in
     ;;
 
   dist)
+    (cd build && cmake -G "Unix Makefiles" -D CMAKE_BUILD_TYPE=Release ..)
     do_dist
     do_doc
     ;;
@@ -102,7 +104,7 @@ case "$target" in
     ;;
 
   install)
-    (cd build && make install)
+    (cd build && cmake -G "Unix Makefiles" -D CMAKE_BUILD_TYPE=Release .. && make install)
     ;;
 
   *)

http://git-wip-us.apache.org/repos/asf/avro/blob/bc3fffb0/lang/c++/impl/Compiler.cc
----------------------------------------------------------------------
diff --git a/lang/c++/impl/Compiler.cc b/lang/c++/impl/Compiler.cc
index 01bbde2..9670844 100644
--- a/lang/c++/impl/Compiler.cc
+++ b/lang/c++/impl/Compiler.cc
@@ -162,9 +162,10 @@ static void assertType(const Entity& e, EntityType et)
 
 static vector<uint8_t> toBin(const std::string& s)
 {
-    vector<uint8_t> result;
-    result.resize(s.size());
-    std::copy(s.c_str(), s.c_str() + s.size(), &result[0]);
+    vector<uint8_t> result(s.size());
+    if (s.size() > 0) {
+        std::copy(s.c_str(), s.c_str() + s.size(), &result[0]);
+    }
     return result;
 }
 

http://git-wip-us.apache.org/repos/asf/avro/blob/bc3fffb0/lang/c++/test/CompilerTests.cc
----------------------------------------------------------------------
diff --git a/lang/c++/test/CompilerTests.cc b/lang/c++/test/CompilerTests.cc
new file mode 100644
index 0000000..a40d6a1
--- /dev/null
+++ b/lang/c++/test/CompilerTests.cc
@@ -0,0 +1,70 @@
+/**
+ * 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 <sstream>
+
+#include <boost/test/included/unit_test_framework.hpp>
+#include <boost/test/unit_test.hpp>
+
+#include "Compiler.hh"
+#include "ValidSchema.hh"
+
+// Assert that empty defaults don't make json schema compilation violate bounds
+// checks, as they did in AVRO-1853. Please note that on Linux bounds are only
+// checked in Debug builds (CMAKE_BUILD_TYPE=Debug).
+void testEmptyBytesDefault()
+{
+    std::string input = "{\n\
+    \"type\": \"record\",\n\
+    \"name\": \"testrecord\",\n\
+    \"fields\": [\n\
+        {\n\
+            \"name\": \"testbytes\",\n\
+            \"type\": \"bytes\",\n\
+            \"default\": \"\"\n\
+        }\n\
+    ]\n\
+}\n\
+";
+    std::string expected = "{\n\
+    \"type\": \"record\",\n\
+    \"name\": \"testrecord\",\n\
+    \"fields\": [\n\
+        {\n\
+            \"name\": \"testbytes\",\n\
+            \"type\": \"bytes\"\n\
+        }\n\
+    ]\n\
+}\n\
+";
+
+    avro::ValidSchema schema = avro::compileJsonSchemaFromString(input);
+    std::ostringstream actual;
+    schema.toJson(actual);
+    BOOST_CHECK_EQUAL(expected, actual.str());
+}
+
+boost::unit_test::test_suite*
+init_unit_test_suite(int argc, char* argv[])
+{
+    using namespace boost::unit_test;
+
+    test_suite* ts= BOOST_TEST_SUITE("Avro C++ unit tests for Compiler.cc");
+    ts->add(BOOST_TEST_CASE(&testEmptyBytesDefault));
+    return ts;
+}