Revert "Havardpe/avoid reading past json value in slime json parser"

5 files changed, 8 insertions, 101 deletions

diff --git a/vespalib/src/tests/data/input_reader/input_reader_test.cpp b/vespalib/src/tests/data/input_reader/input_reader_test.cpp
index 535c188d01e..e8098b7e3ea 100644
--- a/vespalib/src/tests/data/input_reader/input_reader_test.cpp
+++ b/vespalib/src/tests/data/input_reader/input_reader_test.cpp
@@ -112,47 +112,4 @@ TEST("expect that obtain does not set failure state on input reader") {
     }
 }
 
-TEST("require that bytes can be unread when appropriate") {
-    const char *data = "12345";
-    MemoryInput memory_input(data);
-    ChunkedInput input(memory_input, 3);
-    InputReader src(input);
-    EXPECT_TRUE(!src.try_unread());
-    EXPECT_EQUAL(src.read(), '1');
-    EXPECT_EQUAL(src.read(), '2');
-    EXPECT_EQUAL(src.read(), '3');
-    EXPECT_TRUE(src.try_unread());
-    EXPECT_TRUE(src.try_unread());
-    EXPECT_TRUE(src.try_unread());
-    EXPECT_TRUE(!src.try_unread());
-    EXPECT_EQUAL(src.read(), '1');
-    EXPECT_EQUAL(src.read(), '2');
-    EXPECT_EQUAL(src.read(), '3');
-    EXPECT_EQUAL(src.read(), '4');
-    EXPECT_TRUE(src.try_unread());
-    EXPECT_TRUE(!src.try_unread());
-    EXPECT_EQUAL(src.read(), '4');
-    EXPECT_EQUAL(src.read(), '5');
-    EXPECT_EQUAL(src.obtain(), 0u);
-    EXPECT_TRUE(!src.try_unread());
-    EXPECT_TRUE(!src.failed());
-}
-
-TEST("require that try read finds eof without failing the reader") {
-    const char *data = "12345";
-    MemoryInput memory_input(data);
-    ChunkedInput input(memory_input, 3);
-    InputReader src(input);
-    EXPECT_EQUAL(src.try_read(), '1');
-    EXPECT_EQUAL(src.try_read(), '2');
-    EXPECT_EQUAL(src.try_read(), '3');
-    EXPECT_EQUAL(src.try_read(), '4');
-    EXPECT_EQUAL(src.try_read(), '5');
-    EXPECT_TRUE(src.try_unread());
-    EXPECT_EQUAL(src.try_read(), '5');
-    EXPECT_EQUAL(src.try_read(), '\0');
-    EXPECT_TRUE(!src.try_unread());
-    EXPECT_TRUE(!src.failed());
-}
-
 TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/vespalib/src/tests/slime/slime_binary_format_test.cpp b/vespalib/src/tests/slime/slime_binary_format_test.cpp
index e6661cbf554..371a843a445 100644
--- a/vespalib/src/tests/slime/slime_binary_format_test.cpp
+++ b/vespalib/src/tests/slime/slime_binary_format_test.cpp
@@ -632,7 +632,8 @@ TEST("testOptionalDecodeOrder") {
 
 Slime from_json(const vespalib::string &json) {
     Slime slime;
-    EXPECT_TRUE(vespalib::slime::JsonFormat::decode(json, slime) > 0);
+    size_t size = vespalib::slime::JsonFormat::decode(json, slime);
+    EXPECT_EQUAL(size, json.size());
     return slime;
 }
 
diff --git a/vespalib/src/tests/slime/slime_json_format_test.cpp b/vespalib/src/tests/slime/slime_json_format_test.cpp
index d1f77f09af1..52e293f4a5e 100644
--- a/vespalib/src/tests/slime/slime_json_format_test.cpp
+++ b/vespalib/src/tests/slime/slime_json_format_test.cpp
@@ -1,14 +1,10 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #include <vespa/vespalib/testkit/test_kit.h>
 #include <vespa/vespalib/data/slime/slime.h>
-#include <vespa/vespalib/data/input.h>
-#include <vespa/vespalib/data/memory_input.h>
 #include <iostream>
 #include <fstream>
 
 using namespace vespalib::slime::convenience;
-using vespalib::Input;
-using vespalib::MemoryInput;
 
 std::string make_json(const Slime &slime, bool compact) {
     vespalib::SimpleBuffer buf;
@@ -64,13 +60,7 @@ std::string json_string(const std::string &str) {
 
 std::string normalize(const std::string &json) {
     Slime slime;
-    EXPECT_TRUE(vespalib::slime::JsonFormat::decode(json, slime) > 0);
-    return make_json(slime, true);
-}
-
-std::string normalize(Input &input) {
-    Slime slime;
-    EXPECT_TRUE(vespalib::slime::JsonFormat::decode(input, slime) > 0);
+    EXPECT_GREATER(vespalib::slime::JsonFormat::decode(json, slime), 0u);
     return make_json(slime, true);
 }
 
@@ -369,19 +359,4 @@ TEST_F("decode bytes not null-terminated", Slime) {
     EXPECT_TRUE(parse_json_bytes(mem, f));
 }
 
-TEST("require that multiple adjacent values can be decoded from a single input") {
-    vespalib::string data("true{}false[]null\"foo\"'bar'1.5null");
-    MemoryInput input(data);
-    EXPECT_EQUAL(std::string("true"), normalize(input));
-    EXPECT_EQUAL(std::string("{}"), normalize(input));
-    EXPECT_EQUAL(std::string("false"), normalize(input));
-    EXPECT_EQUAL(std::string("[]"), normalize(input));
-    EXPECT_EQUAL(std::string("null"), normalize(input));
-    EXPECT_EQUAL(std::string("\"foo\""), normalize(input));
-    EXPECT_EQUAL(std::string("\"bar\""), normalize(input));
-    EXPECT_EQUAL(std::string("1.5"), normalize(input));
-    EXPECT_EQUAL(std::string("null"), normalize(input));
-    EXPECT_EQUAL(input.obtain().size, 0u);
-}
-
 TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/vespalib/src/vespa/vespalib/data/input_reader.h b/vespalib/src/vespa/vespalib/data/input_reader.h
index f92ced7e508..b3dfa0d4cd7 100644
--- a/vespalib/src/vespa/vespalib/data/input_reader.h
+++ b/vespalib/src/vespa/vespalib/data/input_reader.h
@@ -74,35 +74,6 @@ public:
     }
 
     /**
-     * Try to read a single byte. This function will not fail the
-     * reader with buffer underflow if eof is reached.
-     *
-     * @return the next input byte, or 0 if eof is reached
-     **/
-    char try_read() {
-        if (__builtin_expect(obtain() > 0, true)) {
-            return _data.data[_pos++];
-        }
-        return 0;
-    }
-
-    /**
-     * Try to unread a single byte. This will work for data that is
-     * read, but not yet evicted. Note that after eof is found (the
-     * obtain function returns 0), unreading will not be possible.
-     *
-     * @return whether unreading could be performed
-     **/
-    bool try_unread() {
-        if (__builtin_expect(_pos > 0, true)) {
-            --_pos;
-            return true;
-        } else {
-            return false;
-        }
-    }
-
-    /**
      * Read a continous sequence of bytes. Bytes within an input chunk
      * will be referenced directly. Reads crossing chunk boundries
      * will result in a gathering copy into a temporary buffer owned
diff --git a/vespalib/src/vespa/vespalib/data/slime/json_format.cpp b/vespalib/src/vespa/vespalib/data/slime/json_format.cpp
index 18bf5289f0d..d6126667d68 100644
--- a/vespalib/src/vespa/vespalib/data/slime/json_format.cpp
+++ b/vespalib/src/vespa/vespalib/data/slime/json_format.cpp
@@ -180,7 +180,11 @@ struct JsonDecoder {
     JsonDecoder(InputReader &reader) : in(reader), c(in.read()), key(), value() {}
 
     void next() {
-        c = in.try_read();
+        if (in.obtain() > 0) {
+            c = in.read();
+        } else {
+            c = 0;
+        }
     }
 
     bool skip(char x) {
@@ -485,7 +489,6 @@ JsonFormat::decode(Input &input, Slime &slime)
     InputReader reader(input);
     JsonDecoder decoder(reader);
     decoder.decodeValue(slime);
-    reader.try_unread();
     if (reader.failed()) {
         slime.wrap("partial_result");
         slime.get().setLong("offending_offset", reader.get_offset());