You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by mp...@apache.org on 2018/05/08 20:48:27 UTC

[1/3] kudu git commit: Add ExtractBool method to JsonReader

Repository: kudu
Updated Branches:
  refs/heads/master e7ee66786 -> d83cd11a9


Add ExtractBool method to JsonReader

This will be used in a follow-up.

Change-Id: I2888fc150aaf3db5db01d6135c9d387db39cc179
Reviewed-on: http://gerrit.cloudera.org:8080/10286
Tested-by: Will Berkeley <wd...@gmail.com>
Reviewed-by: Todd Lipcon <to...@apache.org>


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

Branch: refs/heads/master
Commit: fcb71942daaef4c8bdd60366d6c7a2930fdad116
Parents: e7ee667
Author: Will Berkeley <wd...@apache.org>
Authored: Tue May 1 13:16:29 2018 -0700
Committer: Will Berkeley <wd...@gmail.com>
Committed: Tue May 8 17:23:42 2018 +0000

----------------------------------------------------------------------
 src/kudu/util/jsonreader-test.cc | 33 ++++++++++++++++++++++++++-------
 src/kudu/util/jsonreader.cc      | 14 ++++++++++++++
 src/kudu/util/jsonreader.h       |  4 ++++
 3 files changed, 44 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/fcb71942/src/kudu/util/jsonreader-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/jsonreader-test.cc b/src/kudu/util/jsonreader-test.cc
index d9bf65b..9f62c31 100644
--- a/src/kudu/util/jsonreader-test.cc
+++ b/src/kudu/util/jsonreader-test.cc
@@ -15,7 +15,8 @@
 // specific language governing permissions and limitations
 // under the License.
 
-#include <cstddef>
+#include "kudu/util/jsonreader.h"
+
 #include <cstdint>
 #include <string>
 #include <vector>
@@ -25,7 +26,6 @@
 
 #include "kudu/gutil/integral_types.h"
 #include "kudu/gutil/strings/substitute.h"
-#include "kudu/util/jsonreader.h"
 #include "kudu/util/status.h"
 #include "kudu/util/test_macros.h"
 
@@ -51,6 +51,7 @@ TEST(JsonReaderTest, Empty) {
   ASSERT_OK(r2.Init());
 
   // Not found.
+  ASSERT_TRUE(r.ExtractBool(r.root(), "foo", nullptr).IsNotFound());
   ASSERT_TRUE(r.ExtractInt32(r.root(), "foo", nullptr).IsNotFound());
   ASSERT_TRUE(r.ExtractInt64(r.root(), "foo", nullptr).IsNotFound());
   ASSERT_TRUE(r.ExtractString(r.root(), "foo", nullptr).IsNotFound());
@@ -66,6 +67,7 @@ TEST(JsonReaderTest, Basic) {
   ASSERT_EQ("bar", foo);
 
   // Bad types.
+  ASSERT_TRUE(r.ExtractBool(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractInt32(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractInt64(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObject(r.root(), "foo", nullptr).IsInvalidArgument());
@@ -74,7 +76,8 @@ TEST(JsonReaderTest, Basic) {
 
 TEST(JsonReaderTest, LessBasic) {
   string doc = Substitute(
-      "{ \"small\" : 1, \"big\" : $0, \"null\" : null, \"empty\" : \"\" }", kint64max);
+      "{ \"small\" : 1, \"big\" : $0, \"null\" : null, \"empty\" : \"\", \"bool\" : true }",
+      kint64max);
   JsonReader r(doc);
   ASSERT_OK(r.Init());
   int32_t small;
@@ -88,26 +91,39 @@ TEST(JsonReaderTest, LessBasic) {
   ASSERT_EQ("", str);
   ASSERT_OK(r.ExtractString(r.root(), "empty", &str));
   ASSERT_EQ("", str);
+  bool b;
+  ASSERT_OK(r.ExtractBool(r.root(), "bool", &b));
+  ASSERT_TRUE(b);
 
   // Bad types.
+  ASSERT_TRUE(r.ExtractBool(r.root(), "small", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractString(r.root(), "small", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObject(r.root(), "small", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObjectArray(r.root(), "small", nullptr).IsInvalidArgument());
 
+  ASSERT_TRUE(r.ExtractBool(r.root(), "big", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractInt32(r.root(), "big", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractString(r.root(), "big", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObject(r.root(), "big", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObjectArray(r.root(), "big", nullptr).IsInvalidArgument());
 
+  ASSERT_TRUE(r.ExtractBool(r.root(), "null", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractInt32(r.root(), "null", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractInt64(r.root(), "null", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObject(r.root(), "null", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObjectArray(r.root(), "null", nullptr).IsInvalidArgument());
 
+  ASSERT_TRUE(r.ExtractBool(r.root(), "empty", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractInt32(r.root(), "empty", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractInt64(r.root(), "empty", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObject(r.root(), "empty", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObjectArray(r.root(), "empty", nullptr).IsInvalidArgument());
+
+  ASSERT_TRUE(r.ExtractInt32(r.root(), "bool", nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractInt64(r.root(), "bool", nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractString(r.root(), "bool", nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractObject(r.root(), "bool", nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractObjectArray(r.root(), "bool", nullptr).IsInvalidArgument());
 }
 
 TEST(JsonReaderTest, Objects) {
@@ -123,6 +139,7 @@ TEST(JsonReaderTest, Objects) {
   ASSERT_EQ(1, one);
 
   // Bad types.
+  ASSERT_TRUE(r.ExtractBool(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractInt32(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractInt64(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractString(r.root(), "foo", nullptr).IsInvalidArgument());
@@ -143,10 +160,11 @@ TEST(JsonReaderTest, TopLevelArray) {
   ASSERT_EQ("bar", name);
 
   // Bad types.
-  ASSERT_TRUE(r.ExtractInt32(r.root(), nullptr, NULL).IsInvalidArgument());
-  ASSERT_TRUE(r.ExtractInt64(r.root(), nullptr, NULL).IsInvalidArgument());
-  ASSERT_TRUE(r.ExtractString(r.root(), nullptr, NULL).IsInvalidArgument());
-  ASSERT_TRUE(r.ExtractObject(r.root(), nullptr, NULL).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractBool(r.root(), nullptr, nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractInt32(r.root(), nullptr, nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractInt64(r.root(), nullptr, nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractString(r.root(), nullptr, nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractObject(r.root(), nullptr, nullptr).IsInvalidArgument());
 }
 
 TEST(JsonReaderTest, NestedArray) {
@@ -165,6 +183,7 @@ TEST(JsonReaderTest, NestedArray) {
   }
 
   // Bad types.
+  ASSERT_TRUE(r.ExtractBool(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractInt32(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractInt64(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractString(r.root(), "foo", nullptr).IsInvalidArgument());

http://git-wip-us.apache.org/repos/asf/kudu/blob/fcb71942/src/kudu/util/jsonreader.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/jsonreader.cc b/src/kudu/util/jsonreader.cc
index f65a9f3..acbc869 100644
--- a/src/kudu/util/jsonreader.cc
+++ b/src/kudu/util/jsonreader.cc
@@ -42,6 +42,20 @@ Status JsonReader::Init() {
   return Status::OK();
 }
 
+Status JsonReader::ExtractBool(const Value* object,
+                               const char* field,
+                               bool* result) const {
+  const Value* val;
+  RETURN_NOT_OK(ExtractField(object, field, &val));
+  if (PREDICT_FALSE(!val->IsBool())) {
+    return Status::InvalidArgument(Substitute(
+        "Wrong type during field extraction: expected bool but got $0",
+        val->GetType()));
+  }
+  *result = val->GetBool();
+  return Status::OK();
+}
+
 Status JsonReader::ExtractInt32(const Value* object,
                                 const char* field,
                                 int32_t* result) const {

http://git-wip-us.apache.org/repos/asf/kudu/blob/fcb71942/src/kudu/util/jsonreader.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/jsonreader.h b/src/kudu/util/jsonreader.h
index 4799f73..e389b57 100644
--- a/src/kudu/util/jsonreader.h
+++ b/src/kudu/util/jsonreader.h
@@ -48,6 +48,10 @@ class JsonReader {
   // 'field' is NULL, will try to convert 'object' directly into the
   // desire type.
 
+  Status ExtractBool(const rapidjson::Value* object,
+                     const char* field,
+                     bool* result) const;
+
   Status ExtractInt32(const rapidjson::Value* object,
                       const char* field,
                       int32_t* result) const;


[2/3] kudu git commit: KUDU-2433. Don't try to symbolize a nullptr

Posted by mp...@apache.org.
KUDU-2433. Don't try to symbolize a nullptr

Don't try to symbolize a nullptr when walking the stack. It may be
possible to get a null pc in the case of unwinding through a stack frame
in a library that wasn't built with frame pointers, or if the stack was
corrupted.

Attempting to symbolize a nullptr with this code path will overflow the
null pointer from 0 to UINT64_MAX, which causes a fatal exception in
UBSAN builds.

Change-Id: I77d03c1e1dc2fa8e3ec9a4c48895b44f55f457dc
Reviewed-on: http://gerrit.cloudera.org:8080/10341
Reviewed-by: Todd Lipcon <to...@apache.org>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Mike Percy <mp...@apache.org>


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

Branch: refs/heads/master
Commit: 34421f567a3797acdc22203a81e2956fc946f1a9
Parents: fcb7194
Author: Mike Percy <mp...@apache.org>
Authored: Mon May 7 19:48:45 2018 -0700
Committer: Mike Percy <mp...@apache.org>
Committed: Tue May 8 19:22:48 2018 +0000

----------------------------------------------------------------------
 src/kudu/util/debug-util.cc | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/34421f56/src/kudu/util/debug-util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/debug-util.cc b/src/kudu/util/debug-util.cc
index 93a9543..03556d6 100644
--- a/src/kudu/util/debug-util.cc
+++ b/src/kudu/util/debug-util.cc
@@ -696,7 +696,10 @@ string StackTrace::Symbolize() const {
     //
     // This also ensures that we point at the correct line number when using addr2line
     // on logged stacks.
-    if (google::Symbolize(
+    //
+    // We check that the pc is not 0 to avoid undefined behavior in the case of
+    // invalid unwinding (see KUDU-2433).
+    if (pc && google::Symbolize(
             reinterpret_cast<char *>(pc) - 1, tmp, sizeof(tmp))) {
       symbol = tmp;
     }


[3/3] kudu git commit: [java] Fix ColumnSchema.toString NPE

Posted by mp...@apache.org.
[java] Fix ColumnSchema.toString NPE

ColumnSchema#toString throws an NPE on
non-decimal types because the TypeAttributes are
null. This patch fixes the issue and adds a test.

Change-Id: I0a240088fd52891f11f2090d8c457c19e784b7de
Reviewed-on: http://gerrit.cloudera.org:8080/10343
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Kudu Jenkins


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

Branch: refs/heads/master
Commit: d83cd11a9ae6b6038190244e8882f090d650aebf
Parents: 34421f5
Author: Grant Henke <gr...@apache.org>
Authored: Tue May 8 10:35:21 2018 -0500
Committer: Grant Henke <gr...@apache.org>
Committed: Tue May 8 20:28:03 2018 +0000

----------------------------------------------------------------------
 .../main/java/org/apache/kudu/ColumnSchema.java | 10 ++++-
 .../java/org/apache/kudu/TestColumnSchema.java  | 39 ++++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/d83cd11a/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java b/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
index 0cb7099..6bb8263 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
@@ -227,7 +227,15 @@ public class ColumnSchema {
 
   @Override
   public String toString() {
-    return "Column name: " + name + ", type: " + type.getName() + typeAttributes.toStringForType(type);
+    StringBuilder sb = new StringBuilder();
+    sb.append("Column name: ");
+    sb.append(name);
+    sb.append(", type: ");
+    sb.append(type.getName());
+    if (typeAttributes != null) {
+      sb.append(typeAttributes.toStringForType(type));
+    }
+    return sb.toString();
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/kudu/blob/d83cd11a/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java b/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
new file mode 100644
index 0000000..d2d0710
--- /dev/null
+++ b/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
@@ -0,0 +1,39 @@
+// 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.
+package org.apache.kudu;
+
+import org.apache.kudu.ColumnSchema.ColumnSchemaBuilder;
+import org.apache.kudu.util.DecimalUtil;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class TestColumnSchema {
+
+  @Test
+  public void testToString() {
+    ColumnSchema col1 = new ColumnSchemaBuilder("col1", Type.STRING).build();
+    ColumnSchema col2 = new ColumnSchemaBuilder("col2", Type.INT64).build();
+    ColumnSchema col3 = new ColumnSchemaBuilder("col3", Type.DECIMAL)
+        .typeAttributes(DecimalUtil.typeAttributes(5, 2))
+        .build();
+
+    assertEquals("Column name: col1, type: string", col1.toString());
+    assertEquals("Column name: col2, type: int64", col2.toString());
+    assertEquals("Column name: col3, type: decimal(5, 2)", col3.toString());
+  }
+}