Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Don't print whole json data buffer to errorstream on error
`errorstream` must not be overly verbose as clientside it is directly printed
onto the ingame chat window. These days, the serverlist can contain > 200k bytes,
so better print it to warningstream if the data buffer is too long.
  • Loading branch information
est31 committed Jan 28, 2016
1 parent e52ebda commit 860d70b
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
8 changes: 7 additions & 1 deletion src/convert_json.cpp
Expand Up @@ -52,7 +52,13 @@ Json::Value fetchJsonValue(const std::string &url,
if (!reader.parse(stream, root)) {
errorstream << "URL: " << url << std::endl;
errorstream << "Failed to parse json data " << reader.getFormattedErrorMessages();
errorstream << "data: \"" << fetch_result.data << "\"" << std::endl;
if (fetch_result.data.size() > 100) {
errorstream << "Data (" << fetch_result.data.size()
<< " bytes) printed to warningstream." << std::endl;
warningstream << "data: \"" << fetch_result.data << "\"" << std::endl;
} else {
errorstream << "data: \"" << fetch_result.data << "\"" << std::endl;
}
return Json::Value();
}

Expand Down
10 changes: 8 additions & 2 deletions src/script/lua_api/l_util.cpp
Expand Up @@ -161,8 +161,14 @@ int ModApiUtil::l_parse_json(lua_State *L)
if (!reader.parse(stream, root)) {
errorstream << "Failed to parse json data "
<< reader.getFormattedErrorMessages();
errorstream << "data: \"" << jsonstr << "\""
<< std::endl;
size_t jlen = strlen(jsonstr);
if (jlen > 100) {
errorstream << "Data (" << jlen
<< " bytes) printed to warningstream." << std::endl;
warningstream << "data: \"" << jsonstr << "\"" << std::endl;
} else {
errorstream << "data: \"" << jsonstr << "\"" << std::endl;
}
lua_pushnil(L);
return 1;
}
Expand Down

4 comments on commit 860d70b

@davisonio
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@Fixer-007
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't say for sure, but server browser works much worse now, I see json data in console, but nothing in server browser. I need to refresh a lot to get a list. Or maybe it is my internet.

@rubenwardy
Copy link
Member

Choose a reason for hiding this comment

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

Won't be because of this commit

@est31
Copy link
Contributor Author

@est31 est31 commented on 860d70b Feb 2, 2016

Choose a reason for hiding this comment

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

@Fixer-007 you are experiencing minetest/serverlist#10

Wrong JSON data is not a bug of minetest, but of the server list. It has to ensure that it always serves valid json to all clients.

This commit only ensures that the whole ingame console doesn't get spammed by overly long errorstream messages.

Please sign in to comment.