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

allow to use a lua without debug library #190

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fperrad
Copy link

@fperrad fperrad commented Jun 4, 2021

the use of debug library could be not safe.

this commit protects two calls of debug

there are two other occurences of debug:

  • in http/util/lua, which concerns only PUC Lua 5.1, so it is ok for LuaJIT, Lua 5.2, 5.3 & 5.4
  • in spec/helper.lua, but busted uses heavily debug

Copy link
Owner

@daurnimator daurnimator left a comment

Choose a reason for hiding this comment

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

could be not safe.

Could not be safe in what manner?

http/bit.lua Outdated
@@ -11,7 +11,7 @@ This means we can ignore the differences between bit libraries.
if _VERSION ~= "Lua 5.1" then
-- Lua 5.3+ has built-in bit operators, wrap them in a function.
-- Use debug.getinfo to get correct file+line numbers for loaded snippet
local info = debug.getinfo(1, "Sl")
local info = debug and debug.getinfo(1, "Sl") or { currentline = 14 }
Copy link
Owner

Choose a reason for hiding this comment

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

I don't want to have to keep this line number in sync. Would be better to just set to -1

Copy link
Author

Choose a reason for hiding this comment

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

ok.
-1 is also the value when using bytecode without debug info.

@@ -52,7 +52,7 @@ function http_error_methods:new_traceback(message, stream_error, lvl)
if lvl ~= 0 then
-- COMPAT: should be passing `nil` message (not the empty string)
-- see https://github.com/keplerproject/lua-compat-5.3/issues/16
e.traceback = debug.traceback("", lvl)
e.traceback = debug and debug.traceback("", lvl) or "no debug.traceback"
Copy link
Owner

Choose a reason for hiding this comment

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

I don't consider this valid: a function :new_traceback should always gather a traceback or throw an error

@fperrad
Copy link
Author

fperrad commented Jun 9, 2021

This library provides the functionality of the debug interface (§4.7) to Lua programs. You should exert care when using this library. Several of its functions violate basic assumptions about Lua code (e.g., that variables local to a function cannot be accessed from outside; that userdata metatables cannot be changed by Lua code; that Lua programs do not crash) and therefore can compromise otherwise secure code. Moreover, some functions in this library may be slow.

at least, this library is not recommended.

I want run code in production with a Lua interpreter compiled without this library debug

Copy link
Owner

@daurnimator daurnimator left a comment

Choose a reason for hiding this comment

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

How can this PR be tested to ensure regressions do not happen?

@fperrad
Copy link
Author

fperrad commented Jun 9, 2021

not with the test suite using busted which depends on debug.

@fperrad
Copy link
Author

fperrad commented Jun 9, 2021

@daurnimator
Copy link
Owner

not with the test suite using busted which depends on debug.

I believe busted allows you to modify globals during a test, and will restore them afterwards. You should be able to run debug = nil during a test to remove it?

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.

2 participants