Uploaded image for project: 'phpBB3'
  1. phpBB3
  2. PHPBB3-10020

ACP function validate_range() fails partially on non-32-bit systems

    Details

    • Type: Bug
    • Status: Unverified Fix
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.8
    • Fix Version/s: 3.0.9-RC1
    • Component/s: ACP
    • Labels:
      None

      Description

      The problem is

      array('min' => (int) 0x80000000, 'max' => (int) 0x7fffffff)


      The 'min' part just doesn't work like that on another platform such as 64-bit.

        Issue Links

          Activity

          Hide
          bantu Andreas Fischer added a comment - - edited

          Discovered by unit tests in patch for PHPBB3-9823.

          Show
          bantu Andreas Fischer added a comment - - edited Discovered by unit tests in patch for PHPBB3-9823.
          Hide
          Oleg Oleg [X] (Inactive) added a comment - - edited

          This patch is for develop-olympus. It conflicts with changes made so far in PHPBB3-9823.

          The version for PHPBB3-9823 is here: https://github.com/p/phpbb3/commit/d99e1431abf8684078e5a8a09b5fcfa59e2a9412

          Seeing how PHPBB3-9823 is not merged yet, maybe the best way to proceed here is to merge the develop-olympus fix here first and then squash the develop fix into the appropriate commit on nickvergessen's branch.

          Remember to test this on a 32-bit system prior to merging, and double-check that the magic number is correct.

          It may be worth making the magic number into a named constant, as it is used a number of times in develop (mostly in tests).

          Show
          Oleg Oleg [X] (Inactive) added a comment - - edited This patch is for develop-olympus. It conflicts with changes made so far in PHPBB3-9823. The version for PHPBB3-9823 is here: https://github.com/p/phpbb3/commit/d99e1431abf8684078e5a8a09b5fcfa59e2a9412 Seeing how PHPBB3-9823 is not merged yet, maybe the best way to proceed here is to merge the develop-olympus fix here first and then squash the develop fix into the appropriate commit on nickvergessen's branch. Remember to test this on a 32-bit system prior to merging, and double-check that the magic number is correct. It may be worth making the magic number into a named constant, as it is used a number of times in develop (mostly in tests).
          Hide
          bantu Andreas Fischer added a comment -

          -2147483648 seems to be float for some reason.

          php > var_dump(0x80000000);
          float(2147483648)
          php > var_dump((int) 0x80000000);
          int(-2147483648)
          php > var_dump(-2147483648);
          float(-2147483648)
          php > var_dump(-2147483647);
          int(-2147483647)
          php > var_dump((int) -2147483648);
          int(-2147483648)
          php > var_dump((int) -2147483649);
          int(2147483647)
          

          This is on 32bit.

          Show
          bantu Andreas Fischer added a comment - -2147483648 seems to be float for some reason. php > var_dump(0x80000000); float(2147483648) php > var_dump((int) 0x80000000); int(-2147483648) php > var_dump(-2147483648); float(-2147483648) php > var_dump(-2147483647); int(-2147483647) php > var_dump((int) -2147483648); int(-2147483648) php > var_dump((int) -2147483649); int(2147483647) This is on 32bit.
          Hide
          bantu Andreas Fischer added a comment -

          See http://bugs.php.net/bug.php?id=14134 for why this happens.

          Show
          bantu Andreas Fischer added a comment - See http://bugs.php.net/bug.php?id=14134 for why this happens.
          Hide
          bantu Andreas Fischer added a comment - - edited

          My suggestion would be to cast -2147483648 to (int) and to use the literal 2147483647 (also with an integer casting) instead of (int) 0x7fffffff.

          Show
          bantu Andreas Fischer added a comment - - edited My suggestion would be to cast -2147483648 to (int) and to use the literal 2147483647 (also with an integer casting) instead of (int) 0x7fffffff.
          Hide
          Oleg Oleg [X] (Inactive) added a comment -

          Added some commits. I did not change the max though. For develop version, see https://github.com/p/phpbb3/commits/fixed-ticket%2F9823.

          I am wondering if we should implement range checks differently - essentially, only check against supplied requirements. If only a max is given, then perform max check and do not perform a min check (instead of checking min against INT_MIN). On 64-bit systems it is legitimate to have values exceeding 2^31 for constraints.

          Show
          Oleg Oleg [X] (Inactive) added a comment - Added some commits. I did not change the max though. For develop version, see https://github.com/p/phpbb3/commits/fixed-ticket%2F9823 . I am wondering if we should implement range checks differently - essentially, only check against supplied requirements. If only a max is given, then perform max check and do not perform a min check (instead of checking min against INT_MIN). On 64-bit systems it is legitimate to have values exceeding 2^31 for constraints.

            People

            • Assignee:
              Oleg Oleg [X] (Inactive)
              Reporter:
              bantu Andreas Fischer
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development