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

Update Lua-RapidJSON #1816

Merged
merged 1 commit into from
Jun 7, 2024
Merged

Update Lua-RapidJSON #1816

merged 1 commit into from
Jun 7, 2024

Conversation

NiLuJe
Copy link
Member

@NiLuJe NiLuJe commented Jun 6, 2024

In order to deal with metadata.calibre files we've mistakenly mangled in the past ;).

Re: koreader/koreader#11922 (comment) & koreader/koreader#11922 (comment)


This change is Reviewable

In order to deal with metadata.calibre files we've mistakenly mangled in
the past ;).

Re: koreader/koreader#11922 (comment) & koreader/koreader#11922 (comment)
@NiLuJe
Copy link
Member Author

NiLuJe commented Jun 6, 2024

This is a rebased force push in the koreader-prod branch, see the master branch for individual commits.

TL;DR:

diff --git a/src/calibre.hpp b/src/calibre.hpp
index 83d20bf..5bc1351 100644
--- a/src/calibre.hpp
+++ b/src/calibre.hpp
@@ -118,11 +118,25 @@ namespace calibre {
                bool StartObject() {
                        // We only want the book object @ depth 0 (pre-increment), not any of the nested ones.
                        if (depth++ > 0) {
+                               // NOTE: If we encounter an object as the value of a required_field, the input file is malformed,
+                               //       as none of our required_fields should ever be objects ;).
+                               //       We've seen examples where a required_field that should have been an array is instead described as an empty *object*,
+                               //       c.f., https://github.com/koreader/koreader/pull/11922#issuecomment-2152799500
+                               //       As we've currently got a key at the top of the Lua stack, one that we'll potentially never submit because of the heuristics,
+                               //       pop it *right now* to avoid leaving it dangling and unbalancing the Lua stack.
+                               if (required_field) {
+                                       lua_pop(L, 1);
+                                       // Also, unflag the field as required so as not to submit *anything* from that object, in case it isn't actually empty.
+                                       required_field = false;
+                               }
                                return true;
                        }
 
                        // We know exactly how many key-value pairs we'll need
                        lua_createtable(L, 0, required_fields.size());
+                       // Mark table as a JSON object (used when dumping back to JSON)
+                       luaL_getmetatable(L, "json.object");
+                       lua_setmetatable(L, -2);
 
                        // Switch to the object (i.e., hash) context
                        stack_.push_back(context_);
@@ -160,6 +174,9 @@ namespace calibre {
                        }
 
                        lua_newtable(L);
+                       // Mark table as a JSON array
+                       luaL_getmetatable(L, "json.array");
+                       lua_setmetatable(L, -2);
 
                        // Switch to the array context
                        stack_.push_back(context_);
@@ -205,7 +222,7 @@ namespace calibre {
                                fn_(L, this);
                        }
 
-                       int index_;
+                       rapidjson::SizeType index_;
                        void(*fn_)(lua_State* L, Ctx* ctx);
 
                private:
@@ -229,7 +246,7 @@ namespace calibre {
                };
 
                mutable bool required_field;
-               size_t depth;
+               rapidjson::SizeType depth;
                const std::unordered_set<std::string> required_fields {
                        "authors",
                        "last_modified",
diff --git a/src/values.hpp b/src/values.hpp
index c1e4b72..9afe8bb 100644
--- a/src/values.hpp
+++ b/src/values.hpp
@@ -198,7 +198,7 @@ namespace values {
                                fn_(L, this);
                        }
 
-                       int index_;
+                       rapidjson::SizeType index_;
                        void(*fn_)(lua_State* L, Ctx* ctx);
                private:
                        explicit Ctx(void(*f)(lua_State* L, Ctx* ctx)) : index_(0), fn_(f) {}

@NiLuJe NiLuJe merged commit f43f183 into koreader:master Jun 7, 2024
3 checks passed
NiLuJe added a commit to koreader/koreader that referenced this pull request Jun 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants