apache / openwhisk-apigateway

Apache OpenWhisk API Gateway service for exposing actions as REST interfaces.
https://openwhisk.apache.org/
Apache License 2.0
64 stars 45 forks source link

fix(core): upstream openresty fixes; lua global scope pollution #380

Closed mhamann closed 3 years ago

mhamann commented 3 years ago

This is a rather extensive PR. Apologies for that.

There were a number of issues discovered upon upgrading from OpenResty 1.13.x to 1.15.x or 1.17.x. The primary problem was that module-level functions weren't declared local in previous iterations of apigateway. This didn't seem to matter when we were on OpenResty 1.13.x, although some impact can't be ruled out in high-throughput production environments.

OpenResty 1.15 and 1.17 include newer versions of LuaJIT, which seem to be much more sensitive to these types of scope leaks, perhaps due to improved efficiency. Nothing jumped out at me after scanning the commit logs for LuaJIT, but this seems the most likely cause.

OpenResty 1.15 also began emitting warnings about global scope _G usage, so this PR corrects all of those warnings. Since Lua cares about the order in which things are defined (it's not Javascript, after all), the order of functions across many files has been altered to account for this.

There are some background around why this change is necessary here and here.

Essentially the changes here are:

mhamann commented 3 years ago

The existing tests should ensure that the gateway remains functionally equivalent; however, I do want to see if I can write a test to duplicate the scope failure. I'm going to attempt that in a separate PR, given that this has been well-tested locally and fixes both a functional defect as well as a high-severity security vulnerability (https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-11724).