diff --git a/indra/llimage/llimage.cpp b/indra/llimage/llimage.cpp
index 91fa8c6ad15b8c23ea4e0d4343352cb8861c0338..feb97ec2ab2c5649e09bfbe4f9c4e8c7733e8a44 100644
--- a/indra/llimage/llimage.cpp
+++ b/indra/llimage/llimage.cpp
@@ -942,6 +942,12 @@ void LLImageRaw::clear(U8 r, U8 g, U8 b, U8 a)
{
llassert( getComponents() <= 4 );
// This is fairly bogus, but it'll do for now.
+ if (isBufferInvalid())
+ {
+ LL_WARNS() << "Invalid image buffer" << LL_ENDL;
+ return;
+ }
+
U8 *pos = getData();
U32 x, y;
for (x = 0; x < getWidth(); x++)
@@ -1069,6 +1075,11 @@ void LLImageRaw::composite( LLImageRaw* src )
{
LLImageRaw* dst = this; // Just for clarity.
+ if (!validateSrcAndDst("LLImageRaw::composite", src, dst))
+ {
+ return;
+ }
+
llassert(3 == src->getComponents());
llassert(3 == dst->getComponents());
@@ -1136,7 +1147,6 @@ void LLImageRaw::compositeUnscaled4onto3( LLImageRaw* src )
llassert( (3 == src->getComponents()) || (4 == src->getComponents()) );
llassert( (src->getWidth() == dst->getWidth()) && (src->getHeight() == dst->getHeight()) );
-
U8* src_data = src->getData();
U8* dst_data = dst->getData();
S32 pixels = getWidth() * getHeight();
@@ -1171,6 +1181,11 @@ void LLImageRaw::copyUnscaledAlphaMask( LLImageRaw* src, const LLColor4U& fill)
{
LLImageRaw* dst = this; // Just for clarity.
+ if (!validateSrcAndDst("LLImageRaw::copyUnscaledAlphaMask", src, dst))
+ {
+ return;
+ }
+
llassert( 1 == src->getComponents() );
llassert( 4 == dst->getComponents() );
llassert( (src->getWidth() == dst->getWidth()) && (src->getHeight() == dst->getHeight()) );
@@ -1193,6 +1208,12 @@ void LLImageRaw::copyUnscaledAlphaMask( LLImageRaw* src, const LLColor4U& fill)
// Fill the buffer with a constant color
void LLImageRaw::fill( const LLColor4U& color )
{
+ if (isBufferInvalid())
+ {
+ LL_WARNS() << "Invalid image buffer" << LL_ENDL;
+ return;
+ }
+
S32 pixels = getWidth() * getHeight();
if( 4 == getComponents() )
{
@@ -1231,14 +1252,13 @@ LLPointer<LLImageRaw> LLImageRaw::duplicate()
// Src and dst can be any size. Src and dst can each have 3 or 4 components.
void LLImageRaw::copy(LLImageRaw* src)
{
- if (!src)
+ LLImageRaw* dst = this; // Just for clarity.
+
+ if (!validateSrcAndDst("LLImageRaw::copy", src, dst))
{
- LL_WARNS() << "LLImageRaw::copy called with a null src pointer" << LL_ENDL;
return;
}
- LLImageRaw* dst = this; // Just for clarity.
-
if( (src->getWidth() == dst->getWidth()) && (src->getHeight() == dst->getHeight()) )
{
// No scaling needed
@@ -1365,6 +1385,11 @@ void LLImageRaw::copyScaled( LLImageRaw* src )
{
LLImageRaw* dst = this; // Just for clarity.
+ if (!validateSrcAndDst("LLImageRaw::copyScaled", src, dst))
+ {
+ return;
+ }
+
llassert_always( (1 == src->getComponents()) || (3 == src->getComponents()) || (4 == src->getComponents()) );
llassert_always( src->getComponents() == dst->getComponents() );
@@ -1403,6 +1428,12 @@ bool LLImageRaw::scale( S32 new_width, S32 new_height, bool scale_image_data )
{
llassert((1 == getComponents()) || (3 == getComponents()) || (4 == getComponents()) );
+ if (isBufferInvalid())
+ {
+ LL_WARNS() << "Invalid image buffer" << LL_ENDL;
+ return false;
+ }
+
S32 old_width = getWidth();
S32 old_height = getHeight();
@@ -1692,6 +1723,25 @@ void LLImageRaw::compositeRowScaled4onto3( U8* in, U8* out, S32 in_pixel_len, S3
}
}
+bool LLImageRaw::validateSrcAndDst(std::string func, LLImageRaw* src, LLImageRaw* dst)
+{
+ if (!src || !dst || src->isBufferInvalid() || dst->isBufferInvalid())
+ {
+ LL_WARNS() << func << ": Source: ";
+ if (!src) LL_CONT << "Null pointer";
+ else if (src->isBufferInvalid()) LL_CONT << "Invalid buffer";
+ else LL_CONT << "OK";
+
+ LL_CONT << "; Destination: ";
+ if (!dst) LL_CONT << "Null pointer";
+ else if (dst->isBufferInvalid()) LL_CONT << "Invalid buffer";
+ else LL_CONT << "OK";
+ LL_CONT << "." << LL_ENDL;
+
+ return false;
+ }
+ return true;
+}
//----------------------------------------------------------------------------
diff --git a/indra/llimage/llimage.h b/indra/llimage/llimage.h
index adc650d3604417c68e1f830334ce664cbe25b57e..9cc7431a9cfd069d6e0c31420e6287bc5a117200 100644
--- a/indra/llimage/llimage.h
+++ b/indra/llimage/llimage.h
@@ -277,6 +277,9 @@ class LLImageRaw : public LLImageBase
public:
static S32 sGlobalRawMemory;
static S32 sRawImageCount;
+
+private:
+ bool validateSrcAndDst(std::string func, LLImageRaw* src, LLImageRaw* dst);
};
// Compressed representation of image.
diff --git a/indra/llkdu/llimagej2ckdu.cpp b/indra/llkdu/llimagej2ckdu.cpp
index 9347e51b85d2229c1c701cb67b08936ce496a336..025c77b85e8fc269107e2a8070975d99b15deaf8 100644
--- a/indra/llkdu/llimagej2ckdu.cpp
+++ b/indra/llkdu/llimagej2ckdu.cpp
@@ -31,9 +31,31 @@
#include "llpointer.h"
#include "llmath.h"
#include "llkdumem.h"
+#include "stringize.h"
#include "kdu_block_coding.h"
+#include <stdexcept>
+#include <iostream>
+
+namespace {
+// exception used to keep KDU from terminating entire program -- see comments
+// in LLKDUMessageError::flush()
+struct KDUError: public std::runtime_error
+{
+ KDUError(const std::string& msg): std::runtime_error(msg) {}
+};
+} // anonymous namespace
+
+// stream kdu_dims to std::ostream
+// Turns out this must NOT be in the anonymous namespace!
+inline
+std::ostream& operator<<(std::ostream& out, const kdu_dims& dims)
+{
+ return out << "(" << dims.pos.x << "," << dims.pos.y << "),"
+ "[" << dims.size.x << "x" << dims.size.y << "]";
+}
+
class kdc_flow_control {
public:
@@ -165,9 +187,15 @@ struct LLKDUMessageError : public LLKDUMessage
// terminating handler→flush call."
// So throwing an exception here isn't arbitrary: we MUST throw an
// exception if we want to recover from a KDU error.
+ // Because this confused me: the above quote specifically refers to
+ // the kdu_error class, which is constructed internally within KDU at
+ // the point where a fatal error is discovered and reported. It is NOT
+ // talking about the kdu_message subclass passed to
+ // kdu_customize_errors(). Destroying this static object at program
+ // shutdown will NOT engage the behavior described above.
if (end_of_message)
{
- throw "KDU throwing an exception";
+ throw KDUError("LLKDUMessageError::flush()");
}
}
};
@@ -196,6 +224,10 @@ LLImageJ2CKDU::~LLImageJ2CKDU()
// Stuff for new simple decode
void transfer_bytes(kdu_byte *dest, kdu_line_buf &src, int gap, int precision);
+// This is called by the real (private) initDecode() (keep_codestream true)
+// and getMetadata() methods (keep_codestream false). As far as nat can tell,
+// mode is always MODE_FAST. It was called by findDiscardLevelsBoundaries()
+// as well, when that still existed, with keep_codestream true and MODE_FAST.
void LLImageJ2CKDU::setupCodeStream(LLImageJ2C &base, bool keep_codestream, ECodeStreamMode mode)
{
S32 data_size = base.getDataSize();
@@ -206,6 +238,12 @@ void LLImageJ2CKDU::setupCodeStream(LLImageJ2C &base, bool keep_codestream, ECod
//
mCodeStreamp.reset();
+ // It's not clear to nat under what circumstances we would reuse a
+ // pre-existing LLKDUMemSource instance. As of 2016-08-05, it consists of
+ // two U32s and a pointer, so it's not as if it would be a huge overhead
+ // to allocate a new one every time.
+ // Also -- why is base.getData() tested specifically here? If that returns
+ // NULL, shouldn't we bail out of the whole method?
if (!mInputp && base.getData())
{
// The compressed data has been loaded
@@ -262,13 +300,19 @@ void LLImageJ2CKDU::setupCodeStream(LLImageJ2C &base, bool keep_codestream, ECod
S32 components = mCodeStreamp->get_num_components();
- if (components >= 3)
- { // Check that components have consistent dimensions (for PPM file)
- kdu_dims dims1; mCodeStreamp->get_dims(1,dims1);
- kdu_dims dims2; mCodeStreamp->get_dims(2,dims2);
- if ((dims1 != dims) || (dims2 != dims))
+ // Check that components have consistent dimensions (for PPM file)
+ for (int idx = 1; idx < components; ++idx)
+ {
+ kdu_dims other_dims;
+ mCodeStreamp->get_dims(idx, other_dims);
+ if (other_dims != dims)
{
- LL_ERRS() << "Components don't have matching dimensions!" << LL_ENDL;
+ // This method is only called from methods that catch KDUError.
+ // We want to fail the image load, not crash the viewer.
+ throw KDUError(STRINGIZE("Component " << idx << " dimensions "
+ << other_dims
+ << " do not match component 0 dimensions "
+ << dims << "!"));
}
}
@@ -295,6 +339,9 @@ void LLImageJ2CKDU::cleanupCodeStream()
mTileIndicesp.reset();
}
+// This is the protected virtual method called by LLImageJ2C::initDecode().
+// However, as far as nat can tell, LLImageJ2C::initDecode() is called only by
+// llimage_libtest.cpp's load_image() function. No detectable production use.
bool LLImageJ2CKDU::initDecode(LLImageJ2C &base, LLImageRaw &raw_image, int discard_level, int* region)
{
return initDecode(base,raw_image,0.0f,MODE_FAST,0,4,discard_level,region);
@@ -327,6 +374,9 @@ bool LLImageJ2CKDU::initEncode(LLImageJ2C &base, LLImageRaw &raw_image, int bloc
return true;
}
+// This is the real (private) initDecode() called both by the protected
+// initDecode() method and by decodeImpl(). As far as nat can tell, only the
+// decodeImpl() usage matters for production.
bool LLImageJ2CKDU::initDecode(LLImageJ2C &base, LLImageRaw &raw_image, F32 decode_time, ECodeStreamMode mode, S32 first_channel, S32 max_channel_count, int discard_level, int* region)
{
base.resetLastError();
@@ -384,9 +434,9 @@ bool LLImageJ2CKDU::initDecode(LLImageJ2C &base, LLImageRaw &raw_image, F32 deco
mTPosp->x = 0;
}
}
- catch (const char* msg)
+ catch (const KDUError& msg)
{
- base.setLastError(ll_safe_string(msg));
+ base.setLastError(msg.what());
return false;
}
catch (...)
@@ -480,9 +530,9 @@ bool LLImageJ2CKDU::decodeImpl(LLImageJ2C &base, LLImageRaw &raw_image, F32 deco
return false;
}
}
- catch (const char* msg)
+ catch (const KDUError& msg)
{
- base.setLastError(ll_safe_string(msg));
+ base.setLastError(msg.what());
base.decodeFailed();
cleanupCodeStream();
return true; // done
@@ -673,9 +723,9 @@ bool LLImageJ2CKDU::encodeImpl(LLImageJ2C &base, const LLImageRaw &raw_image, co
base.updateData(); // set width, height
delete[] output_buffer;
}
- catch(const char* msg)
+ catch(const KDUError& msg)
{
- base.setLastError(ll_safe_string(msg));
+ base.setLastError(msg.what());
return false;
}
catch( ... )
@@ -697,9 +747,9 @@ bool LLImageJ2CKDU::getMetadata(LLImageJ2C &base)
setupCodeStream(base, false, MODE_FAST);
return true;
}
- catch (const char* msg)
+ catch (const KDUError& msg)
{
- base.setLastError(ll_safe_string(msg));
+ base.setLastError(msg.what());
return false;
}
catch (...)