Use an all Lua JSON parser

[meyskens]

I do not expect this to be merged but wanted to present it

After further investigation of #84 I found out the issue was cjson but since it was a C-based dependency I gave using an all Lua one a try and it worked. This is an edge case for most users probably so if you want to keep cjson that is fine and I'll just keep using my fork. If you find it worth it might need 1) a better location for json.lua and 2) an error check of a test fixed. (Advice welcome this is my very first Lua code)

My main reason to put it here is to get a record of a fix should more people encounter this issue.

Thanks

[luto]

This should probably be json.decode(j)? :)

[meyskens]

oops :o good I was too tired and did not deploy this toning. Will fix ASAP

[luto]

This looks great! Getting rid of the C dependency would maybe also enable us to resolve #45. Good work.

1. a better location for json.lua

Maybe there is a library we can depend on? Like lua-resty-http in our .rockspec. This would probably be cleaner than maintaining our own copy of the json parser inside auto-ssl.

2. an error check of a test fixed

I'll take a look next week.

[meyskens]

Maybe there is a library we can depend on? Like lua-resty-http in our .rockspec. This would probably be cleaner than maintaining our own copy of the json parser inside auto-ssl.

Was thinking the same but the one i found didn't provide one :/ I don't know the Lua world enough to find a good alternative.

[GUI]

Thanks for putting this together, @meyskens!

@meyskens: Did you install OpenResty on your ARM system, or have you installed nginx and manually installed the ngx_lua module?

As @luto indicated over in #84, cjson should be pre-installed with OpenResty, so I'm curious whether cjson is broken in OpenResty under ARM, or if you have a more manual installation where you might be missing cjson completely (but the error looks more like it's trying to load, but unexpectedly failing).

In any case, this has made me realize we're missing noting lua-cjson as a dependency for manual nginx installations, since we're just assuming cjson is present like it is under OpenResty. Ideally, we could add lua-cjson as a dependency in our luarock file, but the version of lua-cjson avaialble from LuaRocks is older and currently not compatible with OpenResty.

So I think we can try to address this, I'd just like to better understand whether the issues is a missing dependency, or something broken with OpenResty's cjson under ARM (in which case, it might be preferable to try and get this addressed on OpenResty's end).

In terms of the specifics in this pull request, if we do end up wanting to depend on a pure Lua JSON library, dkjson might be an option. It looks like that's a pretty popular pure-Lua JSON library that would be available as a luarocks dependency (so like @luto mentioned, we could add it as a dependency like lua-resty-http, instead of needing to vendor this dependency in our own code base).

The only other thing that comes to mind is the potential performance difference between cjson and a pure JSON library. Since this JSON serialization is only used when things aren't cached, I'm not sure the performance difference would really matter, but if we did care, we could also try preferring cjson and then fallback to the pure Lua one in the event it doesn't exist.

Getting rid of the C dependency would maybe also enable us to resolve #45. Good work.

@luto: Just to clarify, the only C dependency we're directly compiling is sockproc, so that's our primary barrier to OPM. Since OPM is only for OpenResty and OpenResty should already include cjson, we shouldn't necessarily need to worry about this C dependency.

[meyskens]

Did you install OpenResty on your ARM system, or have you installed nginx and manually installed the ngx_lua module? [...]

I compile OpenResty in a multiarch Docker build meaning the exact same code runs (in my case) on x86_64, armhf and arm64. The first 2 run without any issue so a missing part is probably not the case which can indeed mean an issue in either cjson or OpenResty, I filed the issue here first as I wasn't aware that cjson was in OpenResty. Also the error is quite obscure about telling what is exactly wrong :/

In terms of the specifics in this pull request, if we do end up wanting to depend on a pure Lua JSON library, dkjson might be an option

This looks like a good option, I'll look into that

we could also try preferring cjson and then fallback to the pure Lua one in the event it doesn't exist.

It does technically exist I think in the arm64 build but has some issue. But I guess something like try{}catch(){} might catch that error and switch over.

[GUI]

@meyskens: I've implemented this slightly differently with a pluggable json_adapter mechanism in v0.12.0: https://github.com/GUI/lua-resty-auto-ssl#json_adapter We're still defaulting to using cjson, but there's an adapter for dkjson (but you'll need to install dkjson separately), so hopefully that will help your use-case (if not, let me know). Thanks for tracking this down, for the pull request, and ideas!