[skjson] Size-constrained input API

Pass an explicit input size instead of requiring a C string.

Thanks to mtklein's clever trick, this has no measurable perf impact.

Change-Id: I64f210a9f653a78b05ab6b58fa34479504aa35ff
Reviewed-on: https://skia-review.googlesource.com/134940
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
This commit is contained in:
Florin Malita 2018-06-14 12:50:43 -04:00 committed by Skia Commit-Bot
parent 00f4df9022
commit fc792b8718
5 changed files with 50 additions and 41 deletions

View File

@ -326,7 +326,7 @@ private:
class DOM final : public SkNoncopyable {
public:
explicit DOM(const char*);
DOM(const char*, size_t);
const Value& root() const { return fRoot; }

View File

@ -5,20 +5,12 @@
* found in the LICENSE file.
*/
#include "SkAutoMalloc.h"
#include "SkData.h"
#include "SkJSON.h"
#include "SkStream.h"
void FuzzSkJSON(sk_sp<SkData> bytes) {
// TODO: add a size + len skjson::DOM factory?
SkAutoMalloc data(bytes->size() + 1);
auto* c_str = static_cast<char*>(data.get());
memcpy(c_str, bytes->data(), bytes->size());
c_str[bytes->size()] = '\0';
skjson::DOM dom(c_str);
skjson::DOM dom(static_cast<const char*>(bytes->data()), bytes->size());
SkDynamicMemoryWStream wstream;
dom.write(&wstream);
}

View File

@ -11,12 +11,12 @@
#include "SkString.h"
#include <cmath>
#include <tuple>
#include <vector>
namespace skjson {
//#define SK_JSON_REPORT_ERRORS
// #define SK_JSON_REPORT_ERRORS
static_assert( sizeof(Value) == 8, "");
static_assert(alignof(Value) == 8, "");
@ -231,7 +231,21 @@ public:
fScopeStack.reserve(kScopeStackReserve);
}
const Value parse(const char* p) {
const Value parse(const char* p, size_t size) {
if (!size) {
this->error(NullValue(), p, "invalid empty input");
}
const char* p_stop = p + size - 1;
// We're only checking for end-of-stream on object/array close('}',']'),
// so we must trim any whitespace from the buffer tail.
while (p_stop > p && is_ws(*p_stop)) --p_stop;
if (*p_stop != '}' && *p_stop != ']') {
return this->error(NullValue(), p_stop, "invalid top-level value");
}
p = skip_ws(p);
switch (*p) {
@ -335,17 +349,17 @@ public:
pop_common:
SkASSERT(*p == '}' || *p == ']');
++p;
if (fScopeStack.empty()) {
SkASSERT(fValueStack.size() == 1);
// Stop condition: parsed the top level element and there is no trailing garbage.
return *skip_ws(p) == '\0'
// Success condition: parsed the top level element and reached the stop token.
return p == p_stop
? fValueStack.front()
: this->error(NullValue(), p, "trailing root garbage");
: this->error(NullValue(), p + 1, "trailing root garbage");
}
++p;
goto match_post_value;
match_array:
@ -372,19 +386,20 @@ public:
return NullValue();
}
const SkString& getError() const {
return fError;
std::tuple<const char*, const SkString> getError() const {
return std::make_tuple(fErrorToken, fErrorMessage);
}
private:
SkArenaAlloc& fAlloc;
SkArenaAlloc& fAlloc;
static constexpr size_t kValueStackReserve = 256;
static constexpr size_t kScopeStackReserve = 128;
std::vector<Value > fValueStack;
std::vector<intptr_t> fScopeStack;
SkString fError;
const char* fErrorToken = nullptr;
SkString fErrorMessage;
template <typename VectorT>
void popScopeAsVec(size_t scope_start) {
@ -485,9 +500,8 @@ private:
template <typename T>
T error(T&& ret_val, const char* p, const char* msg) {
#if defined(SK_JSON_REPORT_ERRORS)
static constexpr size_t kMaxContext = 128;
fError = SkStringPrintf("%s: >", msg);
fError.append(p, std::min(strlen(p), kMaxContext));
fErrorToken = p;
fErrorMessage.set(msg);
#endif
return ret_val;
}
@ -713,11 +727,11 @@ SkString Value::toString() const {
static constexpr size_t kMinChunkSize = 4096;
DOM::DOM(const char* c_str)
DOM::DOM(const char* data, size_t size)
: fAlloc(kMinChunkSize) {
DOMParser parser(fAlloc);
fRoot = parser.parse(c_str);
fRoot = parser.parse(data, size);
}
void DOM::write(SkWStream* stream) const {

View File

@ -25,27 +25,21 @@ protected:
bool isSuitableFor(Backend backend) override { return backend == kNonRendering_Backend; }
void onPerCanvasPreDraw(SkCanvas*) override {
if (auto stream = SkStream::MakeFromFile(kBenchFile)) {
SkASSERT(stream->hasLength());
fCStringData = SkData::MakeUninitialized(stream->getLength() + 1);
auto* data8 = reinterpret_cast<uint8_t*>(fCStringData->writable_data());
SkAssertResult(stream->read(data8, stream->getLength()) == stream->getLength());
data8[stream->getLength()] = '\0';
} else {
fData = SkData::MakeFromFileName(kBenchFile);
if (!fData) {
SkDebugf("!! Could not open bench file: %s\n", kBenchFile);
}
}
void onPerCanvasPostDraw(SkCanvas*) override {
fCStringData = nullptr;
fData = nullptr;
}
void onDraw(int loops, SkCanvas*) override {
if (!fCStringData) return;
if (!fData) return;
for (int i = 0; i < loops; i++) {
skjson::DOM dom(static_cast<const char*>(fCStringData->data()));
skjson::DOM dom(static_cast<const char*>(fData->data()), fData->size());
if (dom.root().is<skjson::NullValue>()) {
SkDebugf("!! Parsing failed.\n");
return;
@ -54,7 +48,7 @@ protected:
}
private:
sk_sp<SkData> fCStringData;
sk_sp<SkData> fData;
using INHERITED = Benchmark;
};

View File

@ -22,8 +22,14 @@ DEF_TEST(SkJSON_Parse, reporter) {
{ "" , nullptr },
{ "[" , nullptr },
{ "]" , nullptr },
{ "[[]" , nullptr },
{ "[]]" , nullptr },
{ "[]f" , nullptr },
{ "{" , nullptr },
{ "}" , nullptr },
{ "{{}" , nullptr },
{ "{}}" , nullptr },
{ "{}f" , nullptr },
{ "{]" , nullptr },
{ "[}" , nullptr },
{ "1" , nullptr },
@ -51,7 +57,9 @@ DEF_TEST(SkJSON_Parse, reporter) {
{ "{ \"k\" : null \"k\" : 1 }", nullptr },
{ "[]" , "[]" },
{ " \n\r\t [ \n\r\t ] \n\r\t " , "[]" },
{ "[[]]" , "[[]]" },
{ "[ null ]" , "[null]" },
{ "[ true ]" , "[true]" },
{ "[ false ]" , "[false]" },
@ -66,6 +74,7 @@ DEF_TEST(SkJSON_Parse, reporter) {
{ "[ \"123456789\" ]" , "[\"123456789\"]" },
{ "[ null , true, false,0,12.8 ]", "[null,true,false,0,12.8]" },
{ "{}" , "{}" },
{ " \n\r\t { \n\r\t } \n\r\t " , "{}" },
{ "{ \"k\" : null }" , "{\"k\":null}" },
{ "{ \"k1\" : null, \"k2 \":0 }", "{\"k1\":null,\"k2 \":0}" },
@ -89,7 +98,7 @@ DEF_TEST(SkJSON_Parse, reporter) {
};
for (const auto& tst : g_tests) {
DOM dom(tst.in);
DOM dom(tst.in, strlen(tst.in));
const auto success = !dom.root().is<NullValue>();
REPORTER_ASSERT(reporter, success == (tst.out != nullptr));
if (!success) continue;
@ -153,7 +162,7 @@ DEF_TEST(SkJSON_DOM_visit, reporter) {
\"k8\": { \"kk1\": 2, \"kk2\": false, \"kk1\": \"baz\" } \n\
}";
DOM dom(json);
DOM dom(json, strlen(json));
const auto& jroot = dom.root().as<ObjectValue>();
REPORTER_ASSERT(reporter, jroot.is<ObjectValue>());