Bug 154285 - IsNumeric(1,2) returns True, must produce an error
Summary: IsNumeric(1,2) returns True, must produce an error
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:24.2.0 target:24.8.0
Keywords: difficultyBeginner, easyHack, skillCpp
Depends on:
Blocks:
 
Reported: 2023-03-20 12:11 UTC by Mike Kaganski
Modified: 2024-02-13 04:52 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Kaganski 2023-03-20 12:11:40 UTC
IsNumeric (and some other Basic run-time functions) does not fail when called with more arguments than it takes. E.g., this gives True:

  IsNumeric(1,2)

The problem is not only strict correctness: people might actually confuse this notation for testing of a "one and two tenths", while actually it's two integer arguments passed to the function, and the second argument is simply ignored. See e.g. bug 123158 comment 8.

The task is to check all the functions in basic/source/runtime/methods.cxx and basic/source/runtime/methods1.cxx, locate all the function implementations that do not check the upper bound of the number of arguments passed to them (in case of IsNumeric, the implementation would be SbRtl_IsNumeric), check if these functions indeed allow passing arbitrary number of arguments (an example of such a function, that can rightfully take unlimited number of arguments, is Array(), and its implementation SbRtl_Array), and for all functions that *can't* take arbitrary number of arguments, introduce respective check setting an error.

Every fixed function must have a separate commit, in case when it causes a regression that requires a revert (who knows). So this is an easyhack which can be done in parallel, by several people.
Comment 1 Xisco Faulí 2023-03-20 12:29:58 UTC
Moving to NEW
Comment 2 potatochick2020 2023-03-20 13:06:01 UTC
I am interested in taking this.
Comment 3 Commit Notification 2023-10-13 15:05:04 UTC
Aron Budea committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/c4c017d1b8fe30d1ef452782eef71371282deb37

tdf#147132  tdf#154285 Flatten and check param count of IsMissing fn

It will be available in 24.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 4 Commit Notification 2024-02-08 23:30:30 UTC
Aron Budea committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/8318337c3e30075acc72fa8c9d5051c33319fdf5

tdf#147132 tdf#154285 basic: Flatten and check param count of Is* fns

It will be available in 24.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 5 Commit Notification 2024-02-13 04:52:03 UTC
Adam Seskunas committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/95dace2eb1ae7ce2fc000cc67e134b7bfadf2c35

tdf#154285 Check upper bound of arguments in SbRtl_CurDir

It will be available in 24.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.