Fix some range issues and 32-bit issues with avatar generation
Summary:
Ref T12444. A few issues:
- `x % (y - z)` doesn't generate values in the full range: the largest value is never generated. Instead, use `x % (1 + y - z)`.
- `digestToRange(1, count)` never generates 0. After fixing the first bug, it could generate `count`. The range of the arrays is `0..(count-1)`, inclusive. Generate the correct range instead.
- `unpack('L', ...)` can unpack a negative number on a 32-bit system. Use `& 0x7FFFFFFF` to mask off the sign bit so the result is always a positive integer.
- FileFinder might return arbitrary keys, but we rely on sequential keys (0, 1, 2, ...)
Test Plan:
- Used `bin/people profileimage ... --force` to regenerate images.
- Added some debugging to verify that the math seemed to be working.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12444
Differential Revision: https://secure.phabricator.com/D17543
This commit is contained in:
@@ -119,6 +119,7 @@ final class PhabricatorFilesComposeAvatarBuiltinFile
|
||||
foreach ($list as $file) {
|
||||
$map['alphanumeric/'.$file] = $root.$file;
|
||||
}
|
||||
|
||||
return $map;
|
||||
}
|
||||
|
||||
@@ -138,11 +139,11 @@ final class PhabricatorFilesComposeAvatarBuiltinFile
|
||||
$border_seed = $username.'_border';
|
||||
|
||||
$pack_key =
|
||||
PhabricatorHash::digestToRange($pack_seed, 1, $pack_count);
|
||||
PhabricatorHash::digestToRange($pack_seed, 0, $pack_count - 1);
|
||||
$color_key =
|
||||
PhabricatorHash::digestToRange($color_seed, 1, $color_count);
|
||||
PhabricatorHash::digestToRange($color_seed, 0, $color_count - 1);
|
||||
$border_key =
|
||||
PhabricatorHash::digestToRange($border_seed, 1, $border_count);
|
||||
PhabricatorHash::digestToRange($border_seed, 0, $border_count - 1);
|
||||
|
||||
$pack = $pack_map[$pack_key];
|
||||
$icon = 'alphanumeric/'.$pack.'/'.$file.'.png';
|
||||
@@ -188,7 +189,7 @@ final class PhabricatorFilesComposeAvatarBuiltinFile
|
||||
->withFollowSymlinks(false)
|
||||
->find();
|
||||
|
||||
return $map;
|
||||
return array_values($map);
|
||||
}
|
||||
|
||||
public static function getBorderMap() {
|
||||
|
||||
@@ -88,9 +88,10 @@ final class PhabricatorHash extends Phobject {
|
||||
}
|
||||
|
||||
$hash = sha1($string, $raw_output = true);
|
||||
$value = head(unpack('L', $hash));
|
||||
// Make sure this ends up positive, even on 32-bit machines.
|
||||
$value = head(unpack('L', $hash)) & 0x7FFFFFFF;
|
||||
|
||||
return $min + ($value % ($max - $min));
|
||||
return $min + ($value % (1 + $max - $min));
|
||||
}
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user