Thanks, Dag! I'm uploading an updated patch (d5539-1b.diff) which clarifies the javadoc comment. Committed with revision 1221666. Further comments can be addressed in followup patches.
> Nits: Do you need the upper bound in AuthenticationServiceBase#getIntProperty?
You're right, we don't, since both of the current callers pass in Integer.MAX_VALUE. I guess it just felt more natural to specify the accepted range rather than just the lower bound, especially since the method is a general one. It shouldn't cause too much bloat, but I can remove that parameter if you think that would be better.
(On a related note, I started wondering about what would happen if the salt length was set so high that the token wouldn't fit in a varchar, and if we'd need an upper bound. That turned out not to be an issue, as the db properties conglomerate doesn't store the values in varchars. It stores them in UserType columns. I tested with salt length 40000, which results in a token with more than 80000 characters, and that worked fine.)
> Could you test with the salt in the upgrade test by reading the property first?
Yes and no. With non-zero length salts, the stored token will vary between test runs, so in that case we'd need to duplicate the hashing algorithm in the test in order to verify that we get the correct token. But the main purpose of the test case is to verify that the token is stored in the expected format (for which we just need to check the format identifier in the first four characters) and that we're still able to connect after upgrade/downgrade, so we might also run the test case with default salt length and disable the full checks of the stored tokens. Those two alternatives should be more or less equivalent with respect to what we want to test in this test.