Uploaded image for project: 'Commons Validator'
  1. Commons Validator
  2. VALIDATOR-308

Logical errors in util.Flags affecting check of multiple flags as well as flag 64

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.4.0 Release
    • Fix Version/s: 1.4.1 Release
    • Component/s: Framework
    • Labels:
    • Environment:

      any; flawed logic is independent of environment

      Description

      I just came across Validator.util.Flags while trying to avoid writing my own flag class. Two errors caught my eye:

      1) Java uses two's complement representation for its signed primitives. Two important consequences: the highest order bit signifies a negative number; Long.MAX_VALUE is not the value that has all bits set. This affects flag.isOn(long) and flag.turnAllOn() .

      2) If I understand correctly, flag.isOn(long) is supposed to test if flag has ALL bits set which are also set in the long argument. Comparing against 0 is the wrong way to do this, even when correcting for the mistake from 1). By comparing against 0, the test merely checks if the two values have ANY bits in common, which is not what the method's documentation seems to imply.

      These cases don't seem to get much use, otherwise this would be a serious problem. (And already be fixed, is my guess.) But still, it is bad enough to completely ruin the day of anyone who happens to rely on these features, and the poor soul would rightly curse anyone who allowed them to persist in this wretched state.

      After submitting this, I can hopefully attach the patch I made. Actually there are two separate patches: one for the test, which illustrates the problems, and a very simple one for the Flags implementation, which fixes them.

      Cheerio!

        Attachments

        1. flags_impl_patch.diff
          0.7 kB
          Til Boerner
        2. flags_test_patch.diff
          1.0 kB
          Til Boerner

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              tilx Til Boerner
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 0.25h
                0.25h
                Remaining:
                Remaining Estimate - 0.25h
                0.25h
                Logged:
                Time Spent - Not Specified
                Not Specified