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.

Sign up for GitHub

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

Save PackedByteArrays as base64 encoded #89186

Merged
merged 1 commit into from
Apr 10, 2024
Merged

Save PackedByteArrays as base64 encoded #89186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
93 changes: 82 additions & 11 deletions core/variant/variant_parser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#include "variant_parser.h"

#include "core/crypto/crypto_core.h"
#include "core/input/input_event.h"
#include "core/io/resource_loader.h"
#include "core/object/script_language.h"
Expand Down Expand Up @@ -595,6 +596,82 @@ Error VariantParser::_parse_construct(Stream *p_stream, Vector<T> &r_construct,
return OK;
}

Error VariantParser::_parse_byte_array(Stream *p_stream, Vector<uint8_t> &r_construct, int &line, String &r_err_str) {
Token token;
get_token(p_stream, token, line, r_err_str);
if (token.type != TK_PARENTHESIS_OPEN) {
r_err_str = "Expected '(' in constructor";
return ERR_PARSE_ERROR;
}

get_token(p_stream, token, line, r_err_str);
if (token.type == TK_STRING) {
// Base64 encoded array.
String base64_encoded_string = token.value;
int strlen = base64_encoded_string.length();
CharString cstr = base64_encoded_string.ascii();

size_t arr_len = 0;
r_construct.resize(strlen / 4 * 3 + 1);
uint8_t *w = r_construct.ptrw();
Error err = CryptoCore::b64_decode(&w[0], r_construct.size(), &arr_len, (unsigned char *)cstr.get_data(), strlen);
if (err) {
r_err_str = "Invalid base64-encoded string";
return ERR_PARSE_ERROR;
}
r_construct.resize(arr_len);

get_token(p_stream, token, line, r_err_str);
if (token.type != TK_PARENTHESIS_CLOSE) {
r_err_str = "Expected ')' in constructor";
return ERR_PARSE_ERROR;
}

} else if (token.type == TK_NUMBER || token.type == TK_IDENTIFIER) {
// Individual elements.
while (true) {
if (token.type != TK_NUMBER) {
bool valid = false;
if (token.type == TK_IDENTIFIER) {
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if we could do this in a way there is no duplicated code..

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure we can easily do it. The _parse_construct does handle the parenthesis and the elements in the coma-separated values.
We could extract the coma-separated values into a dedicated function and maybe check the parenthesis in another one, but like, the problem is knowing whether or not we have a string vs a first number to decide what to do afterwards. This makes it a bit complicated as get_token consumes to token (so checking for a number means we have to consume the number), so it's hard to check things beforehands.

If we want simpler solution, I can try, at least, to add a _parse_number_or_identifier() or something ? So that at least the number checking can be avoided as duplicated code ?

Copy link
Member

Choose a reason for hiding this comment

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

At worst you can add a FIXME/TODO comment.

double real = stor_fix(token.value);
if (real != -1) {
token.type = TK_NUMBER;
token.value = real;
valid = true;
}
}
if (!valid) {
r_err_str = "Expected number in constructor";
return ERR_PARSE_ERROR;
}
}

r_construct.push_back(token.value);

get_token(p_stream, token, line, r_err_str);

if (token.type == TK_COMMA) {
//do none
} else if (token.type == TK_PARENTHESIS_CLOSE) {
break;
} else {
r_err_str = "Expected ',' or ')' in constructor";
return ERR_PARSE_ERROR;
}

get_token(p_stream, token, line, r_err_str);
}
} else if (token.type == TK_PARENTHESIS_CLOSE) {
// Empty array.
return OK;
} else {
r_err_str = "Expected base64 string, or list of numbers in constructor";
return ERR_PARSE_ERROR;
}

return OK;
}

Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream, int &line, String &r_err_str, ResourceParser *p_res_parser) {
if (token.type == TK_CURLY_BRACKET_OPEN) {
Dictionary d;
Expand Down Expand Up @@ -1148,7 +1225,7 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream,
value = array;
} else if (id == "PackedByteArray" || id == "PoolByteArray" || id == "ByteArray") {
Vector<uint8_t> args;
Error err = _parse_construct<uint8_t>(p_stream, args, line, r_err_str);
Error err = _parse_byte_array(p_stream, args, line, r_err_str);
if (err) {
return err;
}
Expand Down Expand Up @@ -2031,17 +2108,11 @@ Error VariantWriter::write(const Variant &p_variant, StoreStringFunc p_store_str
case Variant::PACKED_BYTE_ARRAY: {
p_store_string_func(p_store_string_ud, "PackedByteArray(");
Vector<uint8_t> data = p_variant;
int len = data.size();
const uint8_t *ptr = data.ptr();

for (int i = 0; i < len; i++) {
if (i > 0) {
p_store_string_func(p_store_string_ud, ", ");
}

p_store_string_func(p_store_string_ud, itos(ptr[i]));
if (data.size() > 0) {
p_store_string_func(p_store_string_ud, "\"");
p_store_string_func(p_store_string_ud, CryptoCore::b64_encode_str(data.ptr(), data.size()));
p_store_string_func(p_store_string_ud, "\"");
}

p_store_string_func(p_store_string_ud, ")");
} break;
case Variant::PACKED_INT32_ARRAY: {
Expand Down
1 change: 1 addition & 0 deletions core/variant/variant_parser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ class VariantParser {

template <class T>
static Error _parse_construct(Stream *p_stream, Vector<T> &r_construct, int &line, String &r_err_str);
static Error _parse_byte_array(Stream *p_stream, Vector<uint8_t> &r_construct, int &line, String &r_err_str);
static Error _parse_enginecfg(Stream *p_stream, Vector<String> &strings, int &line, String &r_err_str);
static Error _parse_dictionary(Dictionary &object, Stream *p_stream, int &line, String &r_err_str, ResourceParser *p_res_parser = nullptr);
static Error _parse_array(Array &array, Stream *p_stream, int &line, String &r_err_str, ResourceParser *p_res_parser = nullptr);
Expand Down
3 changes: 2 additions & 1 deletion scene/resources/resource_format_text.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@

// Version 2: changed names for Basis, AABB, Vectors, etc.
// Version 3: new string ID for ext/subresources, breaks forward compat.
#define FORMAT_VERSION 3
// Version 4: PackedByteArray is now stored as base64 encoded.
#define FORMAT_VERSION 4

#define BINARY_FORMAT_VERSION 4

Expand Down
Loading