feat: release v1.0.6#11
Open
AlinsRan wants to merge 1 commit into
Open
Conversation
Drop the unpinned "lua-cjson" dependency from the rockspecs. The library only requires cjson.safe, which OpenResty always bundles. Declaring lua-cjson as a LuaRocks dependency makes consumers install lua-cjson 2.1.0.10-1 (the latest release on luarocks.org; 2.1.0.11 was never published there) into their rocks tree, where it shadows the OpenResty bundled 2.1.0.11 whenever the tree comes first on lua_package_cpath. 2.1.0.10 misses openresty/lua-cjson#82: the empty_array sentinel is pushed through a 47-bit pointer mask but compared unmasked, so on platforms that map shared libraries above 2^47 (arm64 Linux with 48-bit VA, e.g. Apple Silicon) the encoder drops the value and emits invalid JSON such as {"x":}. See apache/apisix#13593. Removing the declaration only narrows the install set; no module code or API changes. CI has never installed lua-cjson, so the test suite has always run against the bundled version. Signed-off-by: AlinsRan <rongxin@api7.ai>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe PR adds a versioned LuaRocks specification, maps exported modules to implementation files, and updates ChangesPackage metadata and dependencies
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Drop the unpinned
"lua-cjson"dependency from the rockspecs and cut v1.0.6.Problem
The library only ever does
require("cjson.safe"), which OpenResty always bundles. But the rockspec declareslua-cjsonas a LuaRocks dependency, so every consumer installing this rock also pulls lua-cjson 2.1.0.10-1 — the latest release on luarocks.org; 2.1.0.11 was never published there — into their rocks tree.When that tree comes first on the worker's
lua_package_cpath(which is the case for APISIX'sdeps/), the installed 2.1.0.10 shadows OpenResty's bundled 2.1.0.11.2.1.0.10 predates openresty/lua-cjson#82: the
empty_arraysentinel is pushed through a 47-bit pointer mask but compared unmasked. On platforms that map shared libraries above 2^47 — arm64 Linux with 48-bit VA, e.g. Apple Silicon — the comparison fails and the encoder emits nothing for the value, producing invalid JSON:Reported downstream as apache/apisix#13593 (APISIX 3.17.0 ships this rock in
deps/; 3.16.0, which did not depend on this library, has no cjson indeps/).Pinning
lua-cjson >= 2.1.0.11is not an option — that version does not exist on luarocks.org. Removing the declaration is the correct fix: the module we use is guaranteed to be present on OpenResty, and nothing should be installing a second copy of cjson alongside the bundled one.Changes
rockspec/lua-resty-openapi-validator-master-0.1-0.rockspec: drop"lua-cjson"rockspec/lua-resty-openapi-validator-1.0.6-1.rockspec: new, copied from 1.0.5-1 without"lua-cjson"Safety
requireinlib/andt/iscjson.safe; no barerequire("cjson")anywhere in the repo.cjson.decodeandcjson.nullare used — both present in the OpenResty bundled build..github/workflows/test.yml,fuzz.ymlandfuzz-nightly.ymlinstall onlyjsonschemaandlua-resty-radixtree; lua-cjson has never been installed in CI, so the suite has always run against the bundled version. This change makes the declared dependency set match what CI has been exercising.Follow-up
apache/apisix bumps its pin to
lua-resty-openapi-validator = 1.0.6-1once 1.0.6-1 is on luarocks.org.Summary by CodeRabbit
lua-cjsonis bundled with OpenResty and does not need to be installed separately through LuaRocks.lua-cjsonpackage where it is already provided by OpenResty.