Skip to content

Commit

Permalink
SF3065265 Bit masking in Variant.getXXXRef() uses wrong mask allowing…
Browse files Browse the repository at this point in the history 
… more than one type to be seen as the requested type. Code that passed in the correct type always worked but invalid types were not always detected.
  • Loading branch information
clay_shooter committed Nov 8, 2010
1 parent 21d0ef6 commit a5aba32
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 26 deletions.
16 changes: 11 additions & 5 deletions jacob/docs/ReleaseNotes.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,17 @@ <h3>Tracked Changes</h3>
<tr>
<td colspan="2"><b>Bugs</b></td>
</tr>
<tr>
<td width="13%" valign="top">3065265</td>
<td width="87%" valign="top">Bit masking in Variant.getXXXRef() uses wrong mask allowing more than
one type to be seen as the requested type. Code that passed in the correct type always worked
but invalid types were not always detected.(M4)</td>
</tr>
<tr>
<td width="13%" valign="top">2935662</td>
<td width="87%" valign="top">Error handling code crashes because of uninitialized data in Dispatch.cpp
Check for NULL fails. pfnDeferredFillIn pointer is not initialized, but it's not NULL.(M4)</td>
</tr>
<tr>
<td width="13%" valign="top">2819445</td>
<td width="87%" valign="top">SafeArray.fromLongArray fails when using VariantLongInt (M3)</td>
Expand All @@ -32,11 +43,6 @@ <h3>Tracked Changes</h3>
checked on every object creation for users who run in the standard
<i>all classes in ROT</i> mode. (M2)</td>
</tr>
<tr>
<td width="13%" valign="top">2935662</td>
<td width="87%" valign="top">Error handling code crashes because of uninitialized data in Dispatch.cpp
Check for NULL fails. pfnDeferredFillIn pointer is not initialized, but it's not NULL.</td>
</tr>
<tr>
<td width="13%" valign="top">&nbsp;</td>
<td width="87%" valign="top">&nbsp;</td>
Expand Down
44 changes: 24 additions & 20 deletions jacob/src/com/jacob/com/Variant.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,13 @@ public class Variant extends JacobObject {
// VT_CARRARY = 28
// VT_USERDEFINED = 29

/** what is this? VT_TYPEMASK && VT_BSTR_BLOB */
/** what is this? VT_TYPEMASK && VT_BSTR_BLOB 0xfff */
public static final short VariantTypeMask = 4095;

/** variant's type is array VT_ARRAY */
/** variant's type is array VT_ARRAY 0x2000 */
public static final short VariantArray = 8192;

/** variant's type is a reference (to IDispatch?) VT_BYREF */
/** variant's type is a reference (to IDispatch?) VT_BYREF 0x4000 */
public static final short VariantByref = 16384;

/*
Expand Down Expand Up @@ -289,6 +289,7 @@ public Variant changeType(short in) {
*
* @return ?? comment says null?
*/
@Override
public native Object clone();

/**
Expand All @@ -303,6 +304,7 @@ public Variant changeType(short in) {
*
* @see java.lang.Object#finalize()
*/
@Override
protected void finalize() {
safeRelease();
}
Expand Down Expand Up @@ -331,7 +333,7 @@ public boolean getBoolean() {
* if variant is not of the requested type
*/
public boolean getBooleanRef() {
if ((this.getvt() & VariantBoolean) == VariantBoolean
if ((this.getvt() & VariantTypeMask) == VariantBoolean
&& (this.getvt() & VariantByref) == VariantByref) {
return getVariantBooleanRef();
} else {
Expand Down Expand Up @@ -365,7 +367,7 @@ public byte getByte() {
* if variant is not of the requested type
*/
public byte getByteRef() {
if ((this.getvt() & VariantByte) == VariantByte
if ((this.getvt() & VariantTypeMask) == VariantByte
&& (this.getvt() & VariantByref) == VariantByref) {
return getVariantByteRef();
} else {
Expand Down Expand Up @@ -404,7 +406,7 @@ public Currency getCurrency() {
* if variant is not of the requested type
*/
public Currency getCurrencyRef() {
if ((this.getvt() & VariantCurrency) == VariantCurrency
if ((this.getvt() & VariantTypeMask) == VariantCurrency
&& (this.getvt() & VariantByref) == VariantByref) {
return new Currency(getVariantCurrencyRef());
} else {
Expand Down Expand Up @@ -438,7 +440,7 @@ public double getDate() {
* if variant is not of the requested type
*/
public double getDateRef() {
if ((this.getvt() & VariantDate) == VariantDate
if ((this.getvt() & VariantTypeMask) == VariantDate
&& (this.getvt() & VariantByref) == VariantByref) {
return getVariantDateRef();
} else {
Expand Down Expand Up @@ -473,7 +475,7 @@ public BigDecimal getDecimal() {
* if variant is not of the requested type
*/
public BigDecimal getDecimalRef() {
if ((this.getvt() & VariantDecimal) == VariantDecimal
if ((this.getvt() & VariantTypeMask) == VariantDecimal
&& (this.getvt() & VariantByref) == VariantByref) {
return (BigDecimal) (getVariantDecRef());
} else {
Expand All @@ -493,7 +495,7 @@ public BigDecimal getDecimalRef() {
* if wrong variant type
*/
public Dispatch getDispatch() {
if ((this.getvt() & VariantDispatch) == VariantDispatch) {
if (this.getvt() == VariantDispatch) {
return toDispatch();
} else {
throw new IllegalStateException(
Expand All @@ -511,7 +513,7 @@ public Dispatch getDispatch() {
* if variant is not of the requested type
*/
public Dispatch getDispatchRef() {
if ((this.getvt() & VariantDispatch) == VariantDispatch
if ((this.getvt() & VariantTypeMask) == VariantDispatch
&& (this.getvt() & VariantByref) == VariantByref) {
return toDispatch();
} else {
Expand Down Expand Up @@ -544,7 +546,7 @@ public double getDouble() {
* if variant is not of the requested type
*/
public double getDoubleRef() {
if ((this.getvt() & VariantDouble) == VariantDouble
if ((this.getvt() & VariantTypeMask) == VariantDouble
&& (this.getvt() & VariantByref) == VariantByref) {
return getVariantDoubleRef();
} else {
Expand Down Expand Up @@ -589,7 +591,7 @@ public int getError() {
* if variant is not of the requested type
*/
public int getErrorRef() {
if ((this.getvt() & VariantError) == VariantError
if ((this.getvt() & VariantTypeMask) == VariantError
&& (this.getvt() & VariantByref) == VariantByref) {
return getVariantErrorRef();
} else {
Expand Down Expand Up @@ -621,7 +623,7 @@ public float getFloat() {
* if variant is not of the requested type
*/
public float getFloatRef() {
if ((this.getvt() & VariantFloat) == VariantFloat
if ((this.getvt() & VariantTypeMask) == VariantFloat
&& (this.getvt() & VariantByref) == VariantByref) {
return getVariantFloatRef();
} else {
Expand Down Expand Up @@ -659,7 +661,7 @@ public int getInt() {
* if variant is not of the requested type
*/
public int getIntRef() {
if ((this.getvt() & VariantInt) == VariantInt
if ((this.getvt() & VariantTypeMask) == VariantInt
&& (this.getvt() & VariantByref) == VariantByref) {
return getVariantIntRef();
} else {
Expand Down Expand Up @@ -735,7 +737,7 @@ public long getLong() {
* if variant is not of the requested type
*/
public long getLongRef() {
if ((this.getvt() & VariantLongInt) == VariantLongInt
if ((this.getvt() & VariantTypeMask) == VariantLongInt
&& (this.getvt() & VariantByref) == VariantByref) {
return getVariantLongRef();
} else {
Expand Down Expand Up @@ -781,7 +783,7 @@ public short getShort() {
* if variant is not of the requested type
*/
public short getShortRef() {
if ((this.getvt() & VariantShort) == VariantShort
if ((this.getvt() & VariantTypeMask) == VariantShort
&& (this.getvt() & VariantByref) == VariantByref) {
return getVariantShortRef();
} else {
Expand Down Expand Up @@ -815,7 +817,7 @@ public String getString() {
* if variant is not of the requested type
*/
public String getStringRef() {
if ((this.getvt() & VariantString) == VariantString
if ((this.getvt() & VariantTypeMask) == VariantString
&& (this.getvt() & VariantByref) == VariantByref) {
return getVariantStringRef();
} else {
Expand All @@ -834,7 +836,7 @@ public String getStringRef() {
* Variant
*/
public Object getVariant() {
if ((this.getvt() & VariantVariant) == VariantVariant
if ((this.getvt() & VariantTypeMask) == VariantVariant
&& (this.getvt() & VariantByref) == VariantByref) {
if (JacobObject.isDebugEnabled()) {
JacobObject.debug("About to call getVariantVariant()");
Expand Down Expand Up @@ -1607,8 +1609,8 @@ public void putStringRef(String in) {
* A object that is to be referenced by this variant. If
* objectToBeWrapped is already of type Variant, then it is used.
* If objectToBeWrapped is not Variant then
* <code>new Variant(objectToBeWrapped)</code> is called and
* the result is passed into the com layer
* <code>new Variant(objectToBeWrapped)</code> is called and the
* result is passed into the com layer
* @throws IllegalArgumentException
* if inVariant = null or if inVariant is a Varint
*/
Expand Down Expand Up @@ -1874,6 +1876,7 @@ private native void putVariantDecRef(int signum, byte scale, int lo,
*
* @see com.jacob.com.JacobObject#safeRelease()
*/
@Override
public void safeRelease() {
// The well known constants should not be released.
// Unfortunately this doesn't fix any other classes that are
Expand Down Expand Up @@ -2161,6 +2164,7 @@ public short toShort() {
* @throws IllegalStateException
* if there is no underlying windows data structure
*/
@Override
public String toString() {
try {
// see if we are in a legal state
Expand Down
2 changes: 1 addition & 1 deletion jacob/src/com/jacob/com/VariantUtilities.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ protected static Object variantToObject(Variant sourceData) {
break;
case Variant.VariantTypeMask: // 4095
result = new NotImplementedException(
"toJavaObject() Not implemented for VariantTypeMask");
"toJavaObject() Not implemented for VariantBstrBlob/VariantTypeMask");
break;
case Variant.VariantArray: // 8192
result = new NotImplementedException(
Expand Down
37 changes: 37 additions & 0 deletions jacob/unittest/com/jacob/com/VariantTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,7 @@ public boolean isComplete() {
*
* @see java.lang.Runnable#run()
*/
@Override
public void run() {
for (int variantIndex = 0; variantIndex < initialRunSize; variantIndex++) {
try {
Expand All @@ -664,4 +665,40 @@ public void run() {
isComplete = true;
}
}

/**
* there was a bitwise masking error that let booleans be seen as dispatch
* objects Bug Report SF3065265
*/
public void testGetDispatch() {
Variant testVariant = new Variant();
testVariant.putBooleanRef(true);
try {
// throws IllegalStateException if Jacob detects the type
// throws some other bad exception if COM blows up failing the
// conversion
testVariant.getDispatchRef();
fail("Should not have converted boolean to dispatch");
} catch (IllegalStateException e) {
// yeah! can't get dispatch from boolean
}
}

/**
* there was a bitwise masking error that let booleans be seen as dispatch
* objects Bug Report SF3065265
*/
public void testGetError() {
Variant testVariant = new Variant();
testVariant.putErrorRef(3);
try {
// throws IllegalStateException if Jacob detects the type
// throws some other bad exception if COM blows up failing the
// conversion
testVariant.getStringRef();
fail("Should not have converted error to string");
} catch (IllegalStateException e) {
// yeah! can't get dispatch from boolean
}
}
}

0 comments on commit a5aba32

Please sign in to comment.