Page MenuHomePhorge

Explicitly cast "limit" (page size) API parameter to int
AcceptedPublic

Authored by aklapper on Fri, May 3, 16:03.
Tags
None
Referenced Files
Unknown Object (File)
Wed, May 15, 09:37
Unknown Object (File)
Wed, May 15, 07:27
Unknown Object (File)
Sun, May 12, 20:08
Unknown Object (File)
Tue, May 7, 16:16
Unknown Object (File)
Sun, May 5, 23:33
Unknown Object (File)
Sun, May 5, 23:33
Unknown Object (File)
Sun, May 5, 16:56
Unknown Object (File)
Sun, May 5, 01:59

Details

Summary

Do not throw an exception when the limit (page size) Conduit API parameter is a float but explicitly convert to int.
As an admin, I am not interested in having invalid user-committed query data trigger an error in the server logs.

ERROR 8192: Implicit conversion from float to int loses precision at [/var/www/html/phorge/phorge/src/view/control/AphrontCursorPagerView.php:76]

Closes T15810

Test Plan

Call /conduit/method/maniphest.search/ with a float value for the limit field. Check the server logs or DarkConsole.

Diff Detail

Repository
rP Phorge
Branch
apiLimitFloatConvert (branched from master)
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 1217
Build 1217: arc lint + arc unit

Event Timeline

aklapper requested review of this revision.Fri, May 3, 16:03

Thanks!

sgtm

src/view/control/AphrontCursorPagerView.php
78

Feel free to create a $page_size so you avoid to cast twice.

And, you unlock shorter lines and a super-nice small diff :3

This revision is now accepted and ready to land.Sat, May 4, 06:49

Should the conduit handler also check this and throw an exception? Doing so will send back a more informative error to the caller. I did some of this in https://secure.phabricator.com/D21872

I guess that would imply editing 15-20 *ConduitAPIMethod.php files which currently

$limit = $request->getValue('limit');
if (!$limit) {
  $limit = $this->getDefaultLimit();
}

and add something like

if (!is_int($limit) && !ctype_digit($limit)) {
  throw new Exception(pht('Field "limit" must be an integer value.'));
}

? Hmm.