Summary
CefSettings.ColorType's packing constructor computes the ARGB value entirely in
32-bit int arithmetic, then relies on implicit widening to store it into the
long color_value field:
public ColorType(int alpha, int red, int green, int blue) {
color_value = (alpha << 24) | (red << 16) | (green << 8) | (blue << 0);
}
When alpha >= 0x80, (alpha << 24) sets the sign bit of the intermediate int
result, making the whole packed expression a negative int. Java's implicit
int -> long widening then sign-extends that negative value, so color_value
ends up with all of the long's upper 32 bits set to 1 -- not the plain
unsigned 32-bit ARGB value a caller would reasonably expect from a field typed
long specifically to represent one.
Repro
CefSettings settings = new CefSettings();
CefSettings.ColorType color = settings.new ColorType(0xFF, 0x11, 0x22, 0x33);
color.getColor(); // returns 0xFFFFFFFFFF112233L (-16772813), not 0x00000000FF112233L
Low alpha values (< 0x80) are unaffected -- e.g. new ColorType(0x7F, 0x11, 0x22, 0x33)
correctly returns 0x7F112233L.
Impact
Any caller comparing getColor() against an expected unsigned ARGB long constant,
or otherwise treating the field as an unsigned 32-bit color value (as the class's
own Javadoc describes: "32-bit ARGB color value, not premultiplied"), will get
surprising results whenever alpha is >= 0x80 -- which includes the common case of a
fully-opaque color (alpha = 0xFF).
Suggested fix
Mask to 32 bits (or build the value with long-typed intermediate arithmetic) before
assigning to color_value, e.g.:
color_value = ((long) alpha << 24 | (long) red << 16 | (long) green << 8 | blue) & 0xFFFFFFFFL;
Found via
Writing CefSettingsTest.java (new unit tests, coverage-expansion effort tracked in #5)
-- see colorTypeHighAlphaSignExtends(), which documents the current (buggy) behavior
with an explanatory comment rather than silently masking it.
Summary
CefSettings.ColorType's packing constructor computes the ARGB value entirely in32-bit
intarithmetic, then relies on implicit widening to store it into thelong color_valuefield:When
alpha >= 0x80,(alpha << 24)sets the sign bit of the intermediateintresult, making the whole packed expression a negative
int. Java's implicitint->longwidening then sign-extends that negative value, socolor_valueends up with all of the
long's upper 32 bits set to1-- not the plainunsigned 32-bit ARGB value a caller would reasonably expect from a field typed
longspecifically to represent one.Repro
Low alpha values (< 0x80) are unaffected -- e.g.
new ColorType(0x7F, 0x11, 0x22, 0x33)correctly returns
0x7F112233L.Impact
Any caller comparing
getColor()against an expected unsigned ARGBlongconstant,or otherwise treating the field as an unsigned 32-bit color value (as the class's
own Javadoc describes: "32-bit ARGB color value, not premultiplied"), will get
surprising results whenever alpha is >= 0x80 -- which includes the common case of a
fully-opaque color (alpha = 0xFF).
Suggested fix
Mask to 32 bits (or build the value with
long-typed intermediate arithmetic) beforeassigning to
color_value, e.g.:Found via
Writing
CefSettingsTest.java(new unit tests, coverage-expansion effort tracked in #5)-- see
colorTypeHighAlphaSignExtends(), which documents the current (buggy) behaviorwith an explanatory comment rather than silently masking it.