Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A critical fix of library crash and control sequence handling (\n) #5

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@
*.pdb
*.vcxproj
*.rc
/install_manifest.txt
44 changes: 44 additions & 0 deletions SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,55 @@

import os

optCC = 'gcc'
optCXX = 'g++'
optAR = 'ar'
optSTRIP = 'strip'

env = Environment(
ENV=os.environ,
tools=['default', 'mb_install'],
toolpath=['#/../mw-scons-tools', '#/Install/mw-scons-tools'])

debug = ARGUMENTS.get('debug', 0)
target = ARGUMENTS.get('target', 0)
SYSROOT = ARGUMENTS.get('sysroot', 0)
TCPREFIX = ARGUMENTS.get('tcprefix', 0)
TCVERSION = ARGUMENTS.get('tcversion', 0)

if SYSROOT!=0:
sysroot=SYSROOT

if TCPREFIX!=0:
optCC=('%s-' % TCPREFIX) + optCC
optCXX=('%s-' % TCPREFIX) + optCXX
optAR=('%s-' % TCPREFIX) + optAR
optSTRIP=('%s-' % TCPREFIX) + optSTRIP

if TCVERSION!=0:
optCC=optCC + ('-%s' % TCVERSION)
optCXX=optCXX + ('-%s' % TCVERSION)
optAR=optAR + ('-%s' % TCPREFIX)

env_options = {
"CC" : optCC,
"CXX" : optCXX,
"LD" : optCXX,
"AR" : optAR,
"STRIP" : optSTRIP
}

if int(debug):
env.Append(CCFLAGS = '-g')

if target == 'arm':
env.Replace(**env_options)
if SYSROOT!=0:
env.Append(CCFLAGS=['--sysroot=%s' % sysroot],
LINKFLAGS=['--sysroot=%s' % sysroot])
env.Append(CCFLAGS=['-I%s/usr/include/arm-linux-gnueabihf/c++/4.9' % sysroot])
env.Append(CCFLAGS=['-I%s/usr/include/c++/4.9' % sysroot])

env.MBAddStandardCompilerFlags()

env.MBAddIncludePaths([
Expand Down
38 changes: 29 additions & 9 deletions src/main/cpp/jsonreader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,36 @@ void JsonReader::reset() {
void JsonReader::send() {
std::string const jsonText(m_buffer.str());
reset();
if (!jsonText.empty()) {
m_jsonRpcPrivate.jsonReaderCallback(jsonText);

if (jsonText.empty())
return;

if(jsonText.size() == 1){
if( jsonText.find(" ") == 0
|| jsonText.find("\t") == 0
|| jsonText.find("\n") == 0
|| jsonText.find("\r") == 0
)
return;
}
if(jsonText.size() == 2 && jsonText.find("\r\n") == 0 )
return;

m_jsonRpcPrivate.jsonReaderCallback(jsonText);
}

void JsonReader::transition(char const ch) {
void JsonReader::setDelimiter(char ch){
m_packetDelimiter = ch;
}

bool JsonReader::transition(char const ch) {
switch (m_state) {
case S0:
if ('{' == ch || '[' == ch) {
m_state = S1;
m_stack.push(ch);
} else if (' ' != ch || '\t' != ch || '\n' != ch || '\r' != ch) {
send();
} else if (' ' == ch || '\t' == ch || '\n' == ch || '\r' == ch) {
return true;
}
break;

Expand All @@ -59,9 +76,7 @@ void JsonReader::transition(char const ch) {
}
}

if (send) {
this->send();
}
return send;
}
break;

Expand All @@ -77,11 +92,16 @@ void JsonReader::transition(char const ch) {
m_state = S2;
break;
}

return false;
}

void JsonReader::feed(char ch) {
m_buffer << ch;
transition(ch);
if(transition(ch))
send();
else if(m_packetDelimiter == ch) //use delimiter as sync sequence. we assume that json is passed as minified string
return send();
}

void JsonReader::feed(char const * const buffer, std::size_t const length) {
Expand Down
4 changes: 3 additions & 1 deletion src/main/cpp/jsonreader.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,20 @@ class JsonReader : public JsonRpcStream {
JSONRPC_API void feed(char const *, std::size_t);
JSONRPC_API void feed(std::string const &);
JSONRPC_API void feedeof(void);
JSONRPC_API void setDelimiter(char);

private:
enum State { S0, S1, S2, S3 };

void reset(void);
void transition(char ch);
bool transition(char ch);
void send(void);

JsonRpcPrivate & m_jsonRpcPrivate;
State m_state;
std::stack <char> m_stack;
std::ostringstream m_buffer;
char m_packetDelimiter = '\n'; //by default use \n char as sync sequence

// Disable copy constructor and assignment.

Expand Down
6 changes: 3 additions & 3 deletions src/main/cpp/jsonrpcprivate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,9 +295,9 @@ void JsonRpcPrivate::handleResponse(
if (i != m_callbacks.end()) {
// Get the shared pointer to the callback
callback = i->second.lock();
// Remove the weak pointer to the callback
m_callbacks.erase(i);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I have cherry picked this over to our release candidate branch for now, and will look over the rest of this pull request later. We switched over to git flow a while back, so the rest of this will have to go into develop until our next release, and there are a few changes both there and in a feature branch that we are about to merge to develop that will likely conflict with the whitespace parsing changes.

// Remove the weak pointer to the callback
m_callbacks.erase(i);
}

// If the callback is still valid, send the response
Expand All @@ -312,7 +312,7 @@ Json::Value JsonRpcPrivate::handleObject(Json::Value const & jsonObject) {
Json::Value const null;
response = invalidRequest(null);
} else {
Json::Value const id(jsonObject["id"]);
Json::Value const id(jsonObject["id"]); //TODO: handle special responses (e.g. parse errors)
if (isRequest(jsonObject)) {
response = handleRequest(jsonObject, id);
} else if (isResponse(jsonObject)) {
Expand Down