diff options
author | Benoit Germain <benoit.germain@ubisoft.com> | 2024-12-04 18:21:40 +0100 |
---|---|---|
committer | Benoit Germain <benoit.germain@ubisoft.com> | 2024-12-04 18:21:40 +0100 |
commit | a334c7662a1a7be5b79efe72076c49ab7c714070 (patch) | |
tree | 402bb6883a3cf590dc140032f5f697677f5d63fa | |
parent | d8580e14fec64dd5bf3dd492a4811b290cbe4aec (diff) | |
download | lanes-a334c7662a1a7be5b79efe72076c49ab7c714070.tar.gz lanes-a334c7662a1a7be5b79efe72076c49ab7c714070.tar.bz2 lanes-a334c7662a1a7be5b79efe72076c49ab7c714070.zip |
Better validation of lane:cancel() arguments
-rw-r--r-- | src/cancel.cpp | 47 | ||||
-rw-r--r-- | src/lane.cpp | 1 |
2 files changed, 30 insertions, 18 deletions
diff --git a/src/cancel.cpp b/src/cancel.cpp index 73ee6f3..5bba9fc 100644 --- a/src/cancel.cpp +++ b/src/cancel.cpp | |||
@@ -111,7 +111,7 @@ static CancelOp WhichCancelOp(lua_State* const L_, StackIndex const idx_) | |||
111 | auto const _op{ WhichCancelOp(_str) }; | 111 | auto const _op{ WhichCancelOp(_str) }; |
112 | lua_remove(L_, idx_); // argument is processed, remove it | 112 | lua_remove(L_, idx_); // argument is processed, remove it |
113 | if (!_op.has_value()) { | 113 | if (!_op.has_value()) { |
114 | raise_luaL_error(L_, "Invalid cancel option %s", _str.data()); | 114 | raise_luaL_error(L_, "Invalid cancel operation '%s'", _str.data()); |
115 | } | 115 | } |
116 | return _op.value(); | 116 | return _op.value(); |
117 | } | 117 | } |
@@ -142,20 +142,24 @@ LUAG_FUNC(cancel_test) | |||
142 | // bool[,reason] = lane_h:cancel( [cancel_op, hookcount] [, timeout] [, wake_lane]) | 142 | // bool[,reason] = lane_h:cancel( [cancel_op, hookcount] [, timeout] [, wake_lane]) |
143 | LUAG_FUNC(lane_cancel) | 143 | LUAG_FUNC(lane_cancel) |
144 | { | 144 | { |
145 | Lane* const _lane{ ToLane(L_, StackIndex{ 1 }) }; | 145 | Lane* const _lane{ ToLane(L_, StackIndex{ 1 }) }; // L_: lane [cancel_op, hookcount] [, timeout] [, wake_lane] |
146 | CancelOp const _op{ WhichCancelOp(L_, StackIndex{ 2 }) }; // this removes the cancel_op string from the stack | 146 | CancelOp const _op{ WhichCancelOp(L_, StackIndex{ 2 }) }; // L_: lane [hookcount] [, timeout] [, wake_lane] |
147 | 147 | ||
148 | int const _hook_count = std::invoke([_op, L_]() { | 148 | int const _hook_count{ std::invoke([_op, L_]() { |
149 | if (_op.hookMask == LuaHookMask::None) { | 149 | if (_op.hookMask == LuaHookMask::None) { |
150 | // the caller shouldn't have provided a hook count in that case | ||
150 | return 0; | 151 | return 0; |
151 | } | 152 | } |
152 | auto const _hook_count{ static_cast<int>(luaL_checkinteger(L_, 2)) }; | 153 | if (luaG_type(L_, StackIndex{ 2 }) != LuaType::NUMBER) { |
153 | lua_remove(L_, 2); // argument is processed, remove it | 154 | raise_luaL_error(L_, "Hook count expected"); |
155 | } | ||
156 | auto const _hook_count{ static_cast<int>(lua_tointeger(L_, 2)) }; | ||
157 | lua_remove(L_, 2); // argument is processed, remove it // L_: lane [timeout] [, wake_lane] | ||
154 | if (_hook_count < 1) { | 158 | if (_hook_count < 1) { |
155 | raise_luaL_error(L_, "Hook count cannot be < 1"); | 159 | raise_luaL_error(L_, "Hook count cannot be < 1"); |
156 | } | 160 | } |
157 | return _hook_count; | 161 | return _hook_count; |
158 | }); | 162 | }) }; |
159 | 163 | ||
160 | std::chrono::time_point<std::chrono::steady_clock> _until{ std::chrono::time_point<std::chrono::steady_clock>::max() }; | 164 | std::chrono::time_point<std::chrono::steady_clock> _until{ std::chrono::time_point<std::chrono::steady_clock>::max() }; |
161 | if (luaG_type(L_, StackIndex{ 2 }) == LuaType::NUMBER) { // we don't want to use lua_isnumber() because of autocoercion | 165 | if (luaG_type(L_, StackIndex{ 2 }) == LuaType::NUMBER) { // we don't want to use lua_isnumber() because of autocoercion |
@@ -163,36 +167,43 @@ LUAG_FUNC(lane_cancel) | |||
163 | if (duration.count() >= 0.0) { | 167 | if (duration.count() >= 0.0) { |
164 | _until = std::chrono::steady_clock::now() + std::chrono::duration_cast<std::chrono::steady_clock::duration>(duration); | 168 | _until = std::chrono::steady_clock::now() + std::chrono::duration_cast<std::chrono::steady_clock::duration>(duration); |
165 | } else { | 169 | } else { |
166 | raise_luaL_argerror(L_, StackIndex{ 2 }, "duration cannot be < 0"); | 170 | raise_luaL_error(L_, "Duration cannot be < 0"); |
167 | } | 171 | } |
168 | lua_remove(L_, 2); // argument is processed, remove it | 172 | lua_remove(L_, 2); // argument is processed, remove it // L_: lane [wake_lane] |
169 | } else if (lua_isnil(L_, 2)) { // alternate explicit "infinite timeout" by passing nil | 173 | } else if (lua_isnil(L_, 2)) { // alternate explicit "infinite timeout" by passing nil |
170 | lua_remove(L_, 2); // argument is processed, remove it | 174 | lua_remove(L_, 2); // argument is processed, remove it // L_: lane [wake_lane] |
171 | } | 175 | } |
172 | 176 | ||
173 | // we wake by default in "hard" mode (remember that hook is hard too), but this can be turned off if desired | 177 | // we wake by default in "hard" mode (remember that hook is hard too), but this can be turned off if desired |
174 | WakeLane _wake_lane{ (_op.mode == CancelRequest::Hard) ? WakeLane::Yes : WakeLane::No }; | 178 | WakeLane _wake_lane{ (_op.mode == CancelRequest::Hard) ? WakeLane::Yes : WakeLane::No }; |
175 | if (lua_gettop(L_) >= 2) { | 179 | if (lua_gettop(L_) >= 2) { |
176 | if (!lua_isboolean(L_, 2)) { | 180 | if (!lua_isboolean(L_, 2)) { |
177 | raise_luaL_error(L_, "wake_lane argument is not a boolean"); | 181 | raise_luaL_error(L_, "Boolean expected for wake_lane argument, got %s", luaG_typename(L_, StackIndex{ 2 }).data()); |
178 | } | 182 | } |
179 | _wake_lane = lua_toboolean(L_, 2) ? WakeLane::Yes : WakeLane::No; | 183 | _wake_lane = lua_toboolean(L_, 2) ? WakeLane::Yes : WakeLane::No; |
180 | lua_remove(L_, 2); // argument is processed, remove it | 184 | lua_remove(L_, 2); // argument is processed, remove it // L_: lane |
185 | } | ||
186 | |||
187 | // if the caller didn't fumble, we should have removed everything from the stack but the lane itself | ||
188 | if (lua_gettop(L_) > 1) { | ||
189 | raise_luaL_error(L_, "Too many arguments"); | ||
181 | } | 190 | } |
182 | STACK_CHECK_START_REL(L_, 0); | 191 | lua_pop(L_, 1); // L_: |
192 | |||
193 | STACK_CHECK_START_ABS(L_, 0); | ||
183 | switch (_lane->cancel(_op, _until, _wake_lane, _hook_count)) { | 194 | switch (_lane->cancel(_op, _until, _wake_lane, _hook_count)) { |
184 | default: // should never happen unless we added a case and forgot to handle it | 195 | default: // should never happen unless we added a case and forgot to handle it |
185 | raise_luaL_error(L_, "should not get here!"); | 196 | raise_luaL_error(L_, "Should not get here!"); |
186 | break; | 197 | break; |
187 | 198 | ||
188 | case CancelResult::Timeout: | 199 | case CancelResult::Timeout: |
189 | lua_pushboolean(L_, 0); // false | 200 | lua_pushboolean(L_, 0); // L_: false |
190 | luaG_pushstring(L_, "timeout"); // false "timeout" | 201 | luaG_pushstring(L_, "timeout"); // L_: false "timeout" |
191 | break; | 202 | break; |
192 | 203 | ||
193 | case CancelResult::Cancelled: | 204 | case CancelResult::Cancelled: |
194 | lua_pushboolean(L_, 1); // true | 205 | lua_pushboolean(L_, 1); // L_: true |
195 | _lane->pushStatusString(L_); // true "<status>" | 206 | _lane->pushStatusString(L_); // L_: true "<status>" |
196 | break; | 207 | break; |
197 | } | 208 | } |
198 | STACK_CHECK(L_, 2); | 209 | STACK_CHECK(L_, 2); |
diff --git a/src/lane.cpp b/src/lane.cpp index 9ab574e..3f6f792 100644 --- a/src/lane.cpp +++ b/src/lane.cpp | |||
@@ -934,6 +934,7 @@ CancelResult Lane::cancel(CancelOp const op_, std::chrono::time_point<std::chron | |||
934 | return internalCancel(CancelRequest::Soft, until_, wakeLane_); | 934 | return internalCancel(CancelRequest::Soft, until_, wakeLane_); |
935 | } else if (op_.hookMask != LuaHookMask::None) { | 935 | } else if (op_.hookMask != LuaHookMask::None) { |
936 | lua_sethook(L, _cancelHook, static_cast<int>(op_.hookMask), hookCount_); | 936 | lua_sethook(L, _cancelHook, static_cast<int>(op_.hookMask), hookCount_); |
937 | // TODO: maybe we should wake the lane here too, because the hook won't do much if the lane is blocked on a linda | ||
937 | } | 938 | } |
938 | 939 | ||
939 | return internalCancel(CancelRequest::Hard, until_, wakeLane_); | 940 | return internalCancel(CancelRequest::Hard, until_, wakeLane_); |