Merge to XFA: Fix subtle issues in opj_skip_from_memory and add unit tests.

Original Review URL: https://codereview.chromium.org/1016203002
TBR=thestig@chromium.org

Review URL: https://codereview.chromium.org/1027443002

5 files changed, 643 insertions, 118 deletions

diff --git a/BUILD.gn b/BUILD.gn
index 830e7d1309..ab230b44d8 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -1554,6 +1554,7 @@ static_library("xfa") {
 test("pdfium_unittests") {
   sources = [
     "core/src/fpdfapi/fpdf_parser/fpdf_parser_decode_unittest.cpp",
+    "core/src/fxcodec/codec/fx_codec_jpx_unittest.cpp",
     "core/src/fxcrt/fx_basic_bstring_unittest.cpp",
     "core/src/fxcrt/fx_basic_wstring_unittest.cpp",
     "testing/fx_string_testhelpers.cpp",
diff --git a/core/src/fxcodec/codec/codec_int.h b/core/src/fxcodec/codec/codec_int.h
index be71bf8dd9..752eb3f1a5 100644
--- a/core/src/fxcodec/codec/codec_int.h
+++ b/core/src/fxcodec/codec/codec_int.h
@@ -4,9 +4,16 @@
  
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
+#ifndef CORE_SRC_FXCODEC_CODEC_CODEC_INT_H_
+#define CORE_SRC_FXCODEC_CODEC_CODEC_INT_H_
+
 #include <limits.h>
 #include <list>
+
+#include "../../../include/fxcodec/fx_codec.h"
 #include "../jbig2/JBig2_Context.h"
+#include "../fx_libopenjpeg/libopenjpeg20/openjpeg.h"  // For OPJ_SIZE_T.
+
 class CCodec_BasicModule : public ICodec_BasicModule
 {
 public:
@@ -144,67 +151,67 @@ public:
     virtual void*		Start();
     virtual void		Finish(void* pContext);
     virtual void		Input(void* pContext, FX_LPCBYTE src_buf, FX_DWORD src_size);
-    virtual int			ReadHeader(void* pContext, int* width, int* height, int* nComps, CFX_DIBAttribute* pAttribute = NULL);

+    virtual int			ReadHeader(void* pContext, int* width, int* height, int* nComps, CFX_DIBAttribute* pAttribute = NULL);
     virtual FX_BOOL		StartScanline(void* pContext, int down_scale);
     virtual FX_BOOL		ReadScanline(void* pContext, FX_LPBYTE dest_buf);
     virtual FX_DWORD	GetAvailInput(void* pContext, FX_LPBYTE* avail_buf_ptr);
 protected:
     IFX_JpegProvider* m_pExtProvider;
 };
-#define PNG_ERROR_SIZE 256

-class CCodec_PngModule : public ICodec_PngModule

-{

-public:

-    CCodec_PngModule()

-    {

-        FXSYS_memset8(m_szLastError, '\0', PNG_ERROR_SIZE);

-    }

-

-    virtual void*		Start(void* pModule);

-    virtual void		Finish(void* pContext);

-    virtual FX_BOOL		Input(void* pContext, FX_LPCBYTE src_buf, FX_DWORD src_size, CFX_DIBAttribute* pAttribute);

-protected:

-    FX_CHAR				m_szLastError[PNG_ERROR_SIZE];

-};

-class CCodec_GifModule : public ICodec_GifModule

-{

-public:

-    CCodec_GifModule()

-    {

-        FXSYS_memset8(m_szLastError, '\0', 256);

-    }

-    virtual void*		Start(void* pModule);

-    virtual void		Finish(void* pContext);

-    virtual FX_DWORD	GetAvailInput(void* pContext, FX_LPBYTE* avail_buf_ptr);

-    virtual void		Input(void* pContext, FX_LPCBYTE src_buf, FX_DWORD src_size);

-

-    virtual FX_INT32	ReadHeader(void* pContext, int* width, int* height,

-                                   int* pal_num, void** pal_pp, int* bg_index, CFX_DIBAttribute* pAttribute);

-

-    virtual FX_INT32	LoadFrameInfo(void* pContext, int* frame_num);

-

-    virtual FX_INT32	LoadFrame(void* pContext, int frame_num, CFX_DIBAttribute* pAttribute);

-

-protected:

-    FX_CHAR				m_szLastError[256];

-};

-class CCodec_BmpModule : public ICodec_BmpModule

-{

-public:

-    CCodec_BmpModule()

-    {

-        FXSYS_memset8(m_szLastError, '\0', 256);

-    }

-    virtual void*		Start(void* pModule);

-    virtual void		Finish(void* pContext);

-    virtual FX_DWORD	GetAvailInput(void* pContext, FX_LPBYTE* avail_buf_ptr);

-    virtual void		Input(void* pContext, FX_LPCBYTE src_buf, FX_DWORD src_size);

-    virtual FX_INT32	ReadHeader(void* pContext, FX_INT32* width, FX_INT32* height, FX_BOOL* tb_flag, FX_INT32* components, FX_INT32* pal_num, FX_DWORD** pal_pp, CFX_DIBAttribute* pAttribute);

-    virtual FX_INT32	LoadImage(void* pContext);

-

-protected:

-    FX_CHAR				m_szLastError[256];

-};

+#define PNG_ERROR_SIZE 256
+class CCodec_PngModule : public ICodec_PngModule
+{
+public:
+    CCodec_PngModule()
+    {
+        FXSYS_memset8(m_szLastError, '\0', PNG_ERROR_SIZE);
+    }
+
+    virtual void*		Start(void* pModule);
+    virtual void		Finish(void* pContext);
+    virtual FX_BOOL		Input(void* pContext, FX_LPCBYTE src_buf, FX_DWORD src_size, CFX_DIBAttribute* pAttribute);
+protected:
+    FX_CHAR				m_szLastError[PNG_ERROR_SIZE];
+};
+class CCodec_GifModule : public ICodec_GifModule
+{
+public:
+    CCodec_GifModule()
+    {
+        FXSYS_memset8(m_szLastError, '\0', 256);
+    }
+    virtual void*		Start(void* pModule);
+    virtual void		Finish(void* pContext);
+    virtual FX_DWORD	GetAvailInput(void* pContext, FX_LPBYTE* avail_buf_ptr);
+    virtual void		Input(void* pContext, FX_LPCBYTE src_buf, FX_DWORD src_size);
+
+    virtual FX_INT32	ReadHeader(void* pContext, int* width, int* height,
+                                   int* pal_num, void** pal_pp, int* bg_index, CFX_DIBAttribute* pAttribute);
+
+    virtual FX_INT32	LoadFrameInfo(void* pContext, int* frame_num);
+
+    virtual FX_INT32	LoadFrame(void* pContext, int frame_num, CFX_DIBAttribute* pAttribute);
+
+protected:
+    FX_CHAR				m_szLastError[256];
+};
+class CCodec_BmpModule : public ICodec_BmpModule
+{
+public:
+    CCodec_BmpModule()
+    {
+        FXSYS_memset8(m_szLastError, '\0', 256);
+    }
+    virtual void*		Start(void* pModule);
+    virtual void		Finish(void* pContext);
+    virtual FX_DWORD	GetAvailInput(void* pContext, FX_LPBYTE* avail_buf_ptr);
+    virtual void		Input(void* pContext, FX_LPCBYTE src_buf, FX_DWORD src_size);
+    virtual FX_INT32	ReadHeader(void* pContext, FX_INT32* width, FX_INT32* height, FX_BOOL* tb_flag, FX_INT32* components, FX_INT32* pal_num, FX_DWORD** pal_pp, CFX_DIBAttribute* pAttribute);
+    virtual FX_INT32	LoadImage(void* pContext);
+
+protected:
+    FX_CHAR				m_szLastError[256];
+};
 class CCodec_IccModule : public ICodec_IccModule
 {
 public:
@@ -252,14 +259,14 @@ public:
     FX_BOOL		Decode(void* ctx, FX_LPBYTE dest_data, int pitch, FX_BOOL bTranslateColor, FX_LPBYTE offsets);
     void		DestroyDecoder(void* ctx);
 };
-class CCodec_TiffModule : public ICodec_TiffModule

-{

-public:

-    virtual FX_LPVOID 	CreateDecoder(IFX_FileRead* file_ptr);

-    virtual void		GetFrames(FX_LPVOID ctx, FX_INT32& frames);

-    virtual FX_BOOL		LoadFrameInfo(FX_LPVOID ctx, FX_INT32 frame, FX_DWORD& width, FX_DWORD& height, FX_DWORD& comps, FX_DWORD& bpc, CFX_DIBAttribute* pAttribute = NULL);

-    virtual FX_BOOL		Decode(FX_LPVOID ctx, class CFX_DIBitmap* pDIBitmap);

-    virtual void		DestroyDecoder(FX_LPVOID ctx);

+class CCodec_TiffModule : public ICodec_TiffModule
+{
+public:
+    virtual FX_LPVOID 	CreateDecoder(IFX_FileRead* file_ptr);
+    virtual void		GetFrames(FX_LPVOID ctx, FX_INT32& frames);
+    virtual FX_BOOL		LoadFrameInfo(FX_LPVOID ctx, FX_INT32 frame, FX_DWORD& width, FX_DWORD& height, FX_DWORD& comps, FX_DWORD& bpc, CFX_DIBAttribute* pAttribute = NULL);
+    virtual FX_BOOL		Decode(FX_LPVOID ctx, class CFX_DIBitmap* pDIBitmap);
+    virtual void		DestroyDecoder(FX_LPVOID ctx);
 };
 class CPDF_Jbig2Interface : public CFX_Object, public CJBig2_Module
 {
@@ -335,27 +342,45 @@ public:
     std::list<CJBig2_CachePair> m_SymbolDictCache;
 private:
 };
-class CFX_DIBAttributeExif : public IFX_DIBAttributeExif

-{

-public:

-    CFX_DIBAttributeExif();

-    ~CFX_DIBAttributeExif();

-    virtual FX_BOOL		GetInfo(FX_WORD tag, FX_LPVOID val);

-

-    FX_BOOL ParseExif(CFX_MapPtrTemplate<FX_DWORD, FX_LPBYTE>* pHead, FX_LPBYTE data, FX_DWORD len, CFX_MapPtrTemplate<FX_DWORD, FX_LPBYTE>* pVal);

-

-    typedef FX_WORD (*_Read2Bytes)(FX_LPBYTE data);

-    typedef FX_DWORD (*_Read4Bytes)(FX_LPBYTE data);

-    FX_LPBYTE ParseExifIFH(FX_LPBYTE data, FX_DWORD len, _Read2Bytes* pReadWord, _Read4Bytes* pReadDword);

-    FX_BOOL ParseExifIFD(CFX_MapPtrTemplate<FX_DWORD, FX_LPBYTE>* pMap, FX_LPBYTE data, FX_DWORD len);

-

-    FX_LPBYTE			m_pExifData;

-

-    FX_DWORD			m_dwExifDataLen;

-

-    void				clear();

-    _Read2Bytes m_readWord;

-    _Read4Bytes m_readDword;

-    CFX_MapPtrTemplate<FX_DWORD, FX_LPBYTE> m_TagHead;

-    CFX_MapPtrTemplate<FX_DWORD, FX_LPBYTE> m_TagVal;

-};

+class CFX_DIBAttributeExif : public IFX_DIBAttributeExif
+{
+public:
+    CFX_DIBAttributeExif();
+    ~CFX_DIBAttributeExif();
+    virtual FX_BOOL		GetInfo(FX_WORD tag, FX_LPVOID val);
+
+    FX_BOOL ParseExif(CFX_MapPtrTemplate<FX_DWORD, FX_LPBYTE>* pHead, FX_LPBYTE data, FX_DWORD len, CFX_MapPtrTemplate<FX_DWORD, FX_LPBYTE>* pVal);
+
+    typedef FX_WORD (*_Read2Bytes)(FX_LPBYTE data);
+    typedef FX_DWORD (*_Read4Bytes)(FX_LPBYTE data);
+    FX_LPBYTE ParseExifIFH(FX_LPBYTE data, FX_DWORD len, _Read2Bytes* pReadWord, _Read4Bytes* pReadDword);
+    FX_BOOL ParseExifIFD(CFX_MapPtrTemplate<FX_DWORD, FX_LPBYTE>* pMap, FX_LPBYTE data, FX_DWORD len);
+
+    FX_LPBYTE			m_pExifData;
+
+    FX_DWORD			m_dwExifDataLen;
+
+    void				clear();
+    _Read2Bytes m_readWord;
+    _Read4Bytes m_readDword;
+    CFX_MapPtrTemplate<FX_DWORD, FX_LPBYTE> m_TagHead;
+    CFX_MapPtrTemplate<FX_DWORD, FX_LPBYTE> m_TagVal;
+};
+
+struct DecodeData {
+public:
+    DecodeData(unsigned char* src_data, OPJ_SIZE_T src_size) :
+        src_data(src_data), src_size(src_size), offset(0) {
+    }
+    unsigned char* src_data;
+    OPJ_SIZE_T     src_size;
+    OPJ_SIZE_T     offset;
+};
+
+/* Wrappers for C-style callbacks. */
+OPJ_SIZE_T opj_read_from_memory (void* p_buffer, OPJ_SIZE_T nb_bytes,  void* p_user_data);
+OPJ_SIZE_T opj_write_from_memory (void* p_buffer, OPJ_SIZE_T nb_bytes, void* p_user_data);
+OPJ_OFF_T opj_skip_from_memory (OPJ_OFF_T nb_bytes, void* p_user_data);
+OPJ_BOOL opj_seek_from_memory (OPJ_OFF_T nb_bytes, void* p_user_data);
+
+#endif  // CORE_SRC_FXCODEC_CODEC_CODEC_INT_H_
diff --git a/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp b/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp
index 164fd3e715..0ba6830944 100644
--- a/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp
+++ b/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp
@@ -4,10 +4,13 @@
  
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
+#include <limits>
+
 #include "../../../include/fxcodec/fx_codec.h"
 #include "codec_int.h"
 #include "../fx_libopenjpeg/libopenjpeg20/openjpeg.h"
 #include "../lcms2/include/fx_lcms2.h"
+
 static void fx_error_callback(const char *msg, void *client_data)
 {
     (void)client_data;
@@ -20,63 +23,91 @@ static void fx_info_callback(const char *msg, void *client_data)
 {
     (void)client_data;
 }
-struct DecodeData {
-    unsigned char* src_data;
-    OPJ_SIZE_T     src_size;
-    OPJ_SIZE_T     offset;
-};
-static OPJ_SIZE_T opj_read_from_memory (void * p_buffer, OPJ_SIZE_T p_nb_bytes,  void* p_user_data)
+OPJ_SIZE_T opj_read_from_memory(void* p_buffer, OPJ_SIZE_T nb_bytes, void* p_user_data)
 {
     DecodeData* srcData = static_cast<DecodeData*>(p_user_data);
-    if (srcData == NULL || srcData->src_size == 0 || srcData->src_data == NULL || srcData->offset >= srcData->src_size) {
+    if (!srcData || !srcData->src_data || srcData->src_size == 0) {
+        return -1;
+    }
+    // Reads at EOF return an error code.
+    if (srcData->offset >= srcData->src_size) {
         return -1;
     }
     OPJ_SIZE_T bufferLength = srcData->src_size - srcData->offset;
-    OPJ_SIZE_T readlength = p_nb_bytes < bufferLength ? p_nb_bytes : bufferLength;
+    OPJ_SIZE_T readlength = nb_bytes < bufferLength ? nb_bytes : bufferLength;
     memcpy(p_buffer, &srcData->src_data[srcData->offset], readlength);
     srcData->offset += readlength;
     return readlength;
 }
-static OPJ_SIZE_T opj_write_from_memory (void * p_buffer, OPJ_SIZE_T p_nb_bytes, void* p_user_data)
+OPJ_SIZE_T opj_write_from_memory(void* p_buffer, OPJ_SIZE_T nb_bytes, void* p_user_data)
 {
     DecodeData* srcData = static_cast<DecodeData*>(p_user_data);
-    if (srcData == NULL || srcData->src_size == 0 || srcData->src_data == NULL || srcData->offset >= srcData->src_size) {
+    if (!srcData || !srcData->src_data || srcData->src_size == 0) {
+        return -1;
+    }
+    // Writes at EOF return an error code.
+    if (srcData->offset >= srcData->src_size) {
         return -1;
     }
     OPJ_SIZE_T bufferLength = srcData->src_size - srcData->offset;
-    OPJ_SIZE_T writeLength = p_nb_bytes < bufferLength ? p_nb_bytes : bufferLength;
+    OPJ_SIZE_T writeLength = nb_bytes < bufferLength ? nb_bytes : bufferLength;
     memcpy(&srcData->src_data[srcData->offset], p_buffer, writeLength);
     srcData->offset += writeLength;
     return writeLength;
 }
-static OPJ_OFF_T opj_skip_from_memory (OPJ_OFF_T p_nb_bytes, void* p_user_data)
+OPJ_OFF_T opj_skip_from_memory(OPJ_OFF_T nb_bytes, void* p_user_data)
 {
     DecodeData* srcData = static_cast<DecodeData*>(p_user_data);
-    if (srcData == NULL || srcData->src_size == 0 || srcData->src_data == NULL) {
+    if (!srcData || !srcData->src_data || srcData->src_size == 0) {
         return -1;
     }
-    if (srcData->offset >= srcData->src_size) {
-        srcData->offset = srcData->src_size;
-        return p_nb_bytes;
+    // Offsets are signed and may indicate a negative skip. Do not support this
+    // because of the strange return convention where either bytes skipped or
+    // -1 is returned. Following that convention, a successful relative seek of
+    // -1 bytes would be required to to give the same result as the error case.
+    if (nb_bytes < 0) {
+        return -1;
     }
-    OPJ_SIZE_T bufferLength = srcData->src_size - srcData->offset;
-    OPJ_SIZE_T skipLength = p_nb_bytes < bufferLength ? p_nb_bytes : bufferLength;
-    srcData->offset += skipLength;
-    return skipLength;
+    // FIXME: use std::make_unsigned<OPJ_OFF_T>::type once c++11 lib is OK'd.
+    uint64_t unsignedNbBytes = static_cast<uint64_t>(nb_bytes);
+    // Additionally, the offset may take us beyond the range of a size_t (e.g.
+    // 32-bit platforms). If so, just clamp at EOF.
+    if (unsignedNbBytes > std::numeric_limits<OPJ_SIZE_T>::max() - srcData->offset) {
+        srcData->offset = srcData->src_size;
+    } else {
+        OPJ_SIZE_T checkedNbBytes = static_cast<OPJ_SIZE_T>(unsignedNbBytes);
+        // Otherwise, mimic fseek() semantics to always succeed, even past EOF,
+        // clamping at EOF.  We can get away with this since we don't actually
+        // provide negative relative skips from beyond EOF back to inside the
+        // data, which would be the only reason to need to know exactly how far
+        // beyond EOF we are.
+        srcData->offset = std::min(srcData->offset + checkedNbBytes, srcData->src_size);
+    }
+    return nb_bytes;
 }
-static OPJ_BOOL opj_seek_from_memory (OPJ_OFF_T p_nb_bytes, void* p_user_data)
+OPJ_BOOL opj_seek_from_memory(OPJ_OFF_T nb_bytes, void* p_user_data)
 {
     DecodeData* srcData = static_cast<DecodeData*>(p_user_data);
-    if (srcData == NULL || srcData->src_size == 0 || srcData->src_data == NULL) {
+    if (!srcData || !srcData->src_data || srcData->src_size == 0) {
         return OPJ_FALSE;
     }
-    if (srcData->offset >= srcData->src_size) {
-        return OPJ_TRUE;
-    }
-    if (p_nb_bytes >= srcData->src_size) {
+    // Offsets are signed and may indicate a negative position, which would
+    // be before the start of the file. Do not support this.
+    if (nb_bytes < 0) {
         return OPJ_FALSE;
     }
-    srcData->offset = p_nb_bytes;
+    // FIXME: use std::make_unsigned<OPJ_OFF_T>::type once c++11 lib is OK'd.
+    uint64_t unsignedNbBytes = static_cast<uint64_t>(nb_bytes);
+    // Additionally, the offset may take us beyond the range of a size_t (e.g.
+    // 32-bit platforms). If so, just clamp at EOF.
+    if (unsignedNbBytes > std::numeric_limits<OPJ_SIZE_T>::max()) {
+        srcData->offset = srcData->src_size;
+    } else {
+        OPJ_SIZE_T checkedNbBytes = static_cast<OPJ_SIZE_T>(nb_bytes);
+        // Otherwise, mimic fseek() semantics to always succeed, even past EOF,
+        // again clamping at EOF.
+        srcData->offset = std::min(checkedNbBytes, srcData->src_size);
+    }
     return OPJ_TRUE;
 }
 opj_stream_t* fx_opj_stream_create_memory_stream (DecodeData* data,	OPJ_SIZE_T p_size, 	OPJ_BOOL p_is_read_stream)
@@ -591,10 +622,7 @@ FX_BOOL CJPX_Decoder::Init(const unsigned char* src_data, int src_size)
     image = NULL;
     m_SrcData = src_data;
     m_SrcSize = src_size;
-    DecodeData srcData;
-    srcData.offset  = 0;
-    srcData.src_size = src_size;
-    srcData.src_data = const_cast<unsigned char*>(src_data);
+    DecodeData srcData(const_cast<unsigned char*>(src_data), src_size);
     l_stream = fx_opj_stream_create_memory_stream(&srcData, OPJ_J2K_STREAM_CHUNK_SIZE, 1);
     if (l_stream == NULL) {
         return FALSE;
diff --git a/core/src/fxcodec/codec/fx_codec_jpx_unittest.cpp b/core/src/fxcodec/codec/fx_codec_jpx_unittest.cpp
new file mode 100644
index 0000000000..f867788759
--- /dev/null
+++ b/core/src/fxcodec/codec/fx_codec_jpx_unittest.cpp
@@ -0,0 +1,470 @@
+// Copyright 2014 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <stdint.h>
+
+#include <limits>
+
+#include "codec_int.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "../../../testing/fx_string_testhelpers.h"
+
+static const OPJ_OFF_T kSkipError = static_cast<OPJ_OFF_T>(-1);
+static const OPJ_SIZE_T kReadError = static_cast<OPJ_SIZE_T>(-1);
+static const OPJ_SIZE_T kWriteError = static_cast<OPJ_SIZE_T>(-1);
+
+static unsigned char stream_data[] = {
+    0x00, 0x01, 0x02, 0x03,
+    0x84, 0x85, 0x86, 0x87,  // Include some hi-bytes, too.
+};
+
+TEST(fxcodec, DecodeDataNullDecodeData) {
+    unsigned char buffer[16];
+    DecodeData* ptr = nullptr;
+
+    // Error codes, not segvs, should callers pass us a NULL pointer.
+    EXPECT_EQ(kReadError, opj_read_from_memory(buffer, sizeof(buffer), ptr));
+    EXPECT_EQ(kWriteError, opj_write_from_memory(buffer, sizeof(buffer), ptr));
+    EXPECT_EQ(kSkipError, opj_skip_from_memory(1, ptr));
+    EXPECT_FALSE(opj_seek_from_memory(1, ptr));
+}
+
+TEST(fxcodec, DecodeDataNullStream) {
+    DecodeData dd(nullptr, 0);
+    unsigned char buffer[16];
+
+    // Reads of size 0 do nothing but return an error code.
+    memset(buffer, 0xbd, sizeof(buffer));
+    EXPECT_EQ(kReadError, opj_read_from_memory(buffer, 0, &dd));
+    EXPECT_EQ(0xbd, buffer[0]);
+
+    // Reads of nonzero size do nothing but return an error code.
+    memset(buffer, 0xbd, sizeof(buffer));
+    EXPECT_EQ(kReadError, opj_read_from_memory(buffer, sizeof(buffer), &dd));
+    EXPECT_EQ(0xbd, buffer[0]);
+
+    // writes of size 0 do nothing but return an error code.
+    EXPECT_EQ(kReadError, opj_write_from_memory(buffer, 0, &dd));
+
+    // writes of nonzero size do nothing but return an error code.
+    EXPECT_EQ(kReadError, opj_write_from_memory(buffer, sizeof(buffer), &dd));
+
+    // Skips of size 0 always return an error code.
+    EXPECT_EQ(kReadError, opj_skip_from_memory(0, &dd));
+
+    // Skips of nonzero size always return an error code.
+    EXPECT_EQ(kReadError, opj_skip_from_memory(1, &dd));
+
+    // Seeks to 0 offset return in error.
+    EXPECT_FALSE(opj_seek_from_memory(0, &dd));
+
+    // Seeks to non-zero offsets return in error.
+    EXPECT_FALSE(opj_seek_from_memory(1, &dd));
+}
+
+TEST(fxcodec, DecodeDataZeroSize) {
+    DecodeData dd(stream_data, 0);
+    unsigned char buffer[16];
+
+    // Reads of size 0 do nothing but return an error code.
+    memset(buffer, 0xbd, sizeof(buffer));
+    EXPECT_EQ(kReadError, opj_read_from_memory(buffer, 0, &dd));
+    EXPECT_EQ(0xbd, buffer[0]);
+
+    // Reads of nonzero size do nothing but return an error code.
+    memset(buffer, 0xbd, sizeof(buffer));
+    EXPECT_EQ(kReadError, opj_read_from_memory(buffer, sizeof(buffer), &dd));
+    EXPECT_EQ(0xbd, buffer[0]);
+
+    // writes of size 0 do nothing but return an error code.
+    EXPECT_EQ(kReadError, opj_write_from_memory(buffer, 0, &dd));
+
+    // writes of nonzero size do nothing but return an error code.
+    EXPECT_EQ(kReadError, opj_write_from_memory(buffer, sizeof(buffer), &dd));
+
+    // Skips of size 0 always return an error code.
+    EXPECT_EQ(kReadError, opj_skip_from_memory(0, &dd));
+
+    // Skips of nonzero size always return an error code.
+    EXPECT_EQ(kReadError, opj_skip_from_memory(1, &dd));
+
+    // Seeks to 0 offset return in error.
+    EXPECT_FALSE(opj_seek_from_memory(0, &dd));
+
+    // Seeks to non-zero offsets return in error.
+    EXPECT_FALSE(opj_seek_from_memory(1, &dd));
+}
+
+TEST(fxcodec, DecodeDataReadInBounds) {
+    unsigned char buffer[16];
+    {
+        DecodeData dd(stream_data, sizeof(stream_data));
+
+        // Exact sized read in a single call.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(8u, opj_read_from_memory(buffer, sizeof(buffer), &dd));
+        EXPECT_EQ(0x00, buffer[0]);
+        EXPECT_EQ(0x01, buffer[1]);
+        EXPECT_EQ(0x02, buffer[2]);
+        EXPECT_EQ(0x03, buffer[3]);
+        EXPECT_EQ(0x84, buffer[4]);
+        EXPECT_EQ(0x85, buffer[5]);
+        EXPECT_EQ(0x86, buffer[6]);
+        EXPECT_EQ(0x87, buffer[7]);
+        EXPECT_EQ(0xbd, buffer[8]);
+    }
+    {
+        DecodeData dd(stream_data, sizeof(stream_data));
+
+        // Simple read.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(2u, opj_read_from_memory(buffer, 2, &dd));
+        EXPECT_EQ(0x00, buffer[0]);
+        EXPECT_EQ(0x01, buffer[1]);
+        EXPECT_EQ(0xbd, buffer[2]);
+
+        // Read of size 0 doesn't affect things.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(0u, opj_read_from_memory(buffer, 0, &dd));
+        EXPECT_EQ(0xbd, buffer[0]);
+
+        // Read exactly up to end of data.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(6u, opj_read_from_memory(buffer, 6, &dd));
+        EXPECT_EQ(0x02, buffer[0]);
+        EXPECT_EQ(0x03, buffer[1]);
+        EXPECT_EQ(0x84, buffer[2]);
+        EXPECT_EQ(0x85, buffer[3]);
+        EXPECT_EQ(0x86, buffer[4]);
+        EXPECT_EQ(0x87, buffer[5]);
+        EXPECT_EQ(0xbd, buffer[6]);
+
+        // Read of size 0 at EOF is still an error.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(kReadError, opj_read_from_memory(buffer, 0, &dd));
+        EXPECT_EQ(0xbd, buffer[0]);
+    }
+}
+
+TEST(fxcodec, DecodeDataReadBeyondBounds) {
+    unsigned char buffer[16];
+    {
+        DecodeData dd(stream_data, sizeof(stream_data));
+
+        // Read beyond bounds in a single step.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(8u, opj_read_from_memory(buffer, sizeof(buffer) + 1, &dd));
+        EXPECT_EQ(0x00, buffer[0]);
+        EXPECT_EQ(0x01, buffer[1]);
+        EXPECT_EQ(0x02, buffer[2]);
+        EXPECT_EQ(0x03, buffer[3]);
+        EXPECT_EQ(0x84, buffer[4]);
+        EXPECT_EQ(0x85, buffer[5]);
+        EXPECT_EQ(0x86, buffer[6]);
+        EXPECT_EQ(0x87, buffer[7]);
+        EXPECT_EQ(0xbd, buffer[8]);
+    }
+    {
+        DecodeData dd(stream_data, sizeof(stream_data));
+
+        // Read well beyond bounds in a single step.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(8u, opj_read_from_memory(
+            buffer, std::numeric_limits<OPJ_SIZE_T>::max(), &dd));
+        EXPECT_EQ(0x00, buffer[0]);
+        EXPECT_EQ(0x01, buffer[1]);
+        EXPECT_EQ(0x02, buffer[2]);
+        EXPECT_EQ(0x03, buffer[3]);
+        EXPECT_EQ(0x84, buffer[4]);
+        EXPECT_EQ(0x85, buffer[5]);
+        EXPECT_EQ(0x86, buffer[6]);
+        EXPECT_EQ(0x87, buffer[7]);
+        EXPECT_EQ(0xbd, buffer[8]);
+    }
+    {
+        DecodeData dd(stream_data, sizeof(stream_data));
+
+        // Read of size 6 gets first 6 bytes.
+        // rest of buffer intact.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(6u, opj_read_from_memory(buffer, 6, &dd));
+        EXPECT_EQ(0x00, buffer[0]);
+        EXPECT_EQ(0x01, buffer[1]);
+        EXPECT_EQ(0x02, buffer[2]);
+        EXPECT_EQ(0x03, buffer[3]);
+        EXPECT_EQ(0x84, buffer[4]);
+        EXPECT_EQ(0x85, buffer[5]);
+        EXPECT_EQ(0xbd, buffer[6]);
+
+        // Read of size 6 gets remaining two bytes.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(2u, opj_read_from_memory(buffer, 6, &dd));
+        EXPECT_EQ(0x86, buffer[0]);
+        EXPECT_EQ(0x87, buffer[1]);
+        EXPECT_EQ(0xbd, buffer[2]);
+
+        // Read of 6 more gets nothing and leaves rest of buffer intact.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(kReadError, opj_read_from_memory(buffer, 6, &dd));
+        EXPECT_EQ(0xbd, buffer[0]);
+    }
+}
+
+TEST(fxcodec, DecodeDataWriteInBounds) {
+    unsigned char stream[16];
+    static unsigned char buffer_data[] = {
+        0x00, 0x01, 0x02, 0x03, 0x80,
+        0x80, 0x81, 0x82, 0x83, 0x84,
+    };
+    {
+        // Pretend the stream can only hold 4 bytes.
+        DecodeData dd(stream, 4);
+
+        memset(stream, 0xbd, sizeof(stream));
+        EXPECT_EQ(4u, opj_write_from_memory(buffer_data, 4, &dd));
+        EXPECT_EQ(0x00, stream[0]);
+        EXPECT_EQ(0x01, stream[1]);
+        EXPECT_EQ(0x02, stream[2]);
+        EXPECT_EQ(0x03, stream[3]);
+        EXPECT_EQ(0xbd, stream[4]);
+    }
+    {
+        // Pretend the stream can only hold 4 bytes.
+        DecodeData dd(stream, 4);
+
+        memset(stream, 0xbd, sizeof(stream));
+        EXPECT_EQ(2u, opj_write_from_memory(buffer_data, 2, &dd));
+        EXPECT_EQ(2u, opj_write_from_memory(buffer_data, 2, &dd));
+        EXPECT_EQ(0x00, stream[0]);
+        EXPECT_EQ(0x01, stream[1]);
+        EXPECT_EQ(0x00, stream[2]);
+        EXPECT_EQ(0x01, stream[3]);
+        EXPECT_EQ(0xbd, stream[4]);
+    }
+}
+
+TEST(fxcodec, DecodeDataWriteBeyondBounds) {
+    unsigned char stream[16];
+    static unsigned char buffer_data[] = {
+        0x10, 0x11, 0x12, 0x13,
+        0x94, 0x95, 0x96, 0x97,
+    };
+    {
+        // Pretend the stream can only hold 4 bytes.
+        DecodeData dd(stream, 4);
+
+        // Write ending past EOF transfers up til EOF.
+        memset(stream, 0xbd, sizeof(stream));
+        EXPECT_EQ(4u, opj_write_from_memory(buffer_data, 5, &dd));
+        EXPECT_EQ(0x10, stream[0]);
+        EXPECT_EQ(0x11, stream[1]);
+        EXPECT_EQ(0x12, stream[2]);
+        EXPECT_EQ(0x13, stream[3]);
+        EXPECT_EQ(0xbd, stream[4]);
+
+        // Subsequent writes fail.
+        memset(stream, 0xbd, sizeof(stream));
+        EXPECT_EQ(kWriteError, opj_write_from_memory(buffer_data, 5, &dd));
+        EXPECT_EQ(0xbd, stream[0]);
+    }
+    {
+        // Pretend the stream can only hold 4 bytes.
+        DecodeData dd(stream, 4);
+
+        // Write ending past EOF (two steps) transfers up til EOF.
+        memset(stream, 0xbd, sizeof(stream));
+        EXPECT_EQ(2u, opj_write_from_memory(buffer_data, 2, &dd));
+        EXPECT_EQ(2u, opj_write_from_memory(buffer_data, 4, &dd));
+        EXPECT_EQ(0x10, stream[0]);
+        EXPECT_EQ(0x11, stream[1]);
+        EXPECT_EQ(0x10, stream[2]);
+        EXPECT_EQ(0x11, stream[3]);
+        EXPECT_EQ(0xbd, stream[4]);
+
+        // Subsequent writes fail.
+        memset(stream, 0xbd, sizeof(stream));
+        EXPECT_EQ(kWriteError, opj_write_from_memory(buffer_data, 5, &dd));
+        EXPECT_EQ(0xbd, stream[0]);
+    }
+}
+
+// Note: Some care needs to be taken here because the skip/seek functions
+// take OPJ_OFF_T's as arguments, which are typically a signed type.
+TEST(fxcodec, DecodeDataSkip) {
+    unsigned char buffer[16];
+    {
+        DecodeData dd(stream_data, sizeof(stream_data));
+
+        // Skiping within buffer is allowed.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(1, opj_skip_from_memory(1, &dd));
+        EXPECT_EQ(1u, opj_read_from_memory(buffer, 1, &dd));
+        EXPECT_EQ(0x01, buffer[0]);
+        EXPECT_EQ(0xbd, buffer[1]);
+
+        // Skiping 0 bytes changes nothing.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(0, opj_skip_from_memory(0, &dd));
+        EXPECT_EQ(1u, opj_read_from_memory(buffer, 1, &dd));
+        EXPECT_EQ(0x02, buffer[0]);
+        EXPECT_EQ(0xbd, buffer[1]);
+
+        // Skiping to EOS-1 is possible.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(4, opj_skip_from_memory(4, &dd));
+        EXPECT_EQ(1u, opj_read_from_memory(buffer, 1, &dd));
+        EXPECT_EQ(0x87, buffer[0]);
+        EXPECT_EQ(0xbd, buffer[1]);
+
+        // Next read fails.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(kReadError, opj_read_from_memory(buffer, 1, &dd));
+        EXPECT_EQ(0xbd, buffer[0]);
+    }
+    {
+        DecodeData dd(stream_data, sizeof(stream_data));
+
+        // Skiping directly to EOS is allowed.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(8, opj_skip_from_memory(8, &dd));
+
+        // Next read fails.
+        EXPECT_EQ(kReadError, opj_read_from_memory(buffer, 1, &dd));
+        EXPECT_EQ(0xbd, buffer[0]);
+    }
+    {
+        DecodeData dd(stream_data, sizeof(stream_data));
+
+        // Skipping beyond end of stream is allowed and returns full distance.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(9, opj_skip_from_memory(9, &dd));
+
+        // Next read fails.
+        EXPECT_EQ(kReadError, opj_read_from_memory(buffer, 1, &dd));
+        EXPECT_EQ(0xbd, buffer[0]);
+    }
+    {
+        DecodeData dd(stream_data, sizeof(stream_data));
+
+        // Skipping way beyond EOS is allowd, doesn't wrap, and returns
+        // full distance.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(4, opj_skip_from_memory(4, &dd));
+        EXPECT_EQ(std::numeric_limits<OPJ_OFF_T>::max(), opj_skip_from_memory(
+            std::numeric_limits<OPJ_OFF_T>::max(), &dd));
+
+        // Next read fails. If it succeeds, it may mean we wrapped.
+        EXPECT_EQ(kReadError, opj_read_from_memory(buffer, 1, &dd));
+        EXPECT_EQ(0xbd, buffer[0]);
+    }
+    {
+        DecodeData dd(stream_data, sizeof(stream_data));
+
+        // Negative skip within buffer not is allowed, position unchanged.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(4, opj_skip_from_memory(4, &dd));
+        EXPECT_EQ(kSkipError, opj_skip_from_memory(-2, &dd));
+
+        // Next read succeeds as if nothing has happenned.
+        EXPECT_EQ(1u, opj_read_from_memory(buffer, 1, &dd));
+        EXPECT_EQ(0x84, buffer[0]);
+        EXPECT_EQ(0xbd, buffer[1]);
+
+        // Negative skip before buffer is not allowed, position unchanged.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(kSkipError, opj_skip_from_memory(-4, &dd));
+
+        // Next read succeeds as if nothing has happenned.
+        EXPECT_EQ(1u, opj_read_from_memory(buffer, 1, &dd));
+        EXPECT_EQ(0x85, buffer[0]);
+        EXPECT_EQ(0xbd, buffer[1]);
+    }
+    {
+        DecodeData dd(stream_data, sizeof(stream_data));
+
+        // Negative skip way before buffer is not allowed, doesn't wrap
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(4, opj_skip_from_memory(4, &dd));
+        EXPECT_EQ(kSkipError, opj_skip_from_memory(
+            std::numeric_limits<OPJ_OFF_T>::min(), &dd));
+
+        // Next read succeeds. If it fails, it may mean we wrapped.
+        EXPECT_EQ(1, opj_read_from_memory(buffer, 1, &dd));
+        EXPECT_EQ(0x84, buffer[0]);
+        EXPECT_EQ(0xbd, buffer[1]);
+    }
+    {
+        DecodeData dd(stream_data, sizeof(stream_data));
+
+        // Negative skip after EOS isn't alowed, still EOS.
+        memset(buffer, 0xbd, sizeof(buffer));
+        EXPECT_EQ(8, opj_skip_from_memory(8, &dd));
+        EXPECT_EQ(kSkipError, opj_skip_from_memory(-4, &dd));
+
+        // Next read fails.
+        EXPECT_EQ(kReadError, opj_read_from_memory(buffer, 1, &dd));
+        EXPECT_EQ(0xbd, buffer[0]);
+    }
+
+}
+
+TEST(fxcodec, DecodeDataSeek) {
+    unsigned char buffer[16];
+    DecodeData dd(stream_data, sizeof(stream_data));
+
+    // Seeking within buffer is allowed and read succeeds
+    memset(buffer, 0xbd, sizeof(buffer));
+    EXPECT_TRUE(opj_seek_from_memory(1, &dd));
+    EXPECT_EQ(1u, opj_read_from_memory(buffer, 1, &dd));
+    EXPECT_EQ(0x01, buffer[0]);
+    EXPECT_EQ(0xbd, buffer[1]);
+
+    // Seeking before start returns error leaving position unchanged.
+    memset(buffer, 0xbd, sizeof(buffer));
+    EXPECT_FALSE(opj_seek_from_memory(-1, &dd));
+    EXPECT_EQ(1, opj_read_from_memory(buffer, 1, &dd));
+    EXPECT_EQ(0x02, buffer[0]);
+    EXPECT_EQ(0xbd, buffer[1]);
+
+    // Seeking way before start returns error leaving position unchanged.
+    memset(buffer, 0xbd, sizeof(buffer));
+    EXPECT_FALSE(opj_seek_from_memory(
+        std::numeric_limits<OPJ_OFF_T>::min(), &dd));
+    EXPECT_EQ(1, opj_read_from_memory(buffer, 1, &dd));
+    EXPECT_EQ(0x03, buffer[0]);
+    EXPECT_EQ(0xbd, buffer[1]);
+
+    // Seeking exactly to EOS is allowed but read fails.
+    memset(buffer, 0xbd, sizeof(buffer));
+    EXPECT_TRUE(opj_seek_from_memory(8, &dd));
+    EXPECT_EQ(kReadError, opj_read_from_memory(buffer, 1, &dd));
+    EXPECT_EQ(0xbd, buffer[0]);
+
+    // Seeking back to zero offset is allowed and read succeeds.
+    memset(buffer, 0xbd, sizeof(buffer));
+    EXPECT_TRUE(opj_seek_from_memory(0, &dd));
+    EXPECT_EQ(1u, opj_read_from_memory(buffer, 1, &dd));
+    EXPECT_EQ(0x00, buffer[0]);
+    EXPECT_EQ(0xbd, buffer[1]);
+
+    // Seeking beyond end of stream is allowed but read fails.
+    memset(buffer, 0xbd, sizeof(buffer));
+    EXPECT_TRUE(opj_seek_from_memory(16, &dd));
+    EXPECT_EQ(kReadError, opj_read_from_memory(buffer, 1, &dd));
+    EXPECT_EQ(0xbd, buffer[0]);
+
+    // Seeking within buffer after seek past EOF restores good state.
+    memset(buffer, 0xbd, sizeof(buffer));
+    EXPECT_TRUE(opj_seek_from_memory(4, &dd));
+    EXPECT_EQ(1u, opj_read_from_memory(buffer, 1, &dd));
+    EXPECT_EQ(0x84, buffer[0]);
+    EXPECT_EQ(0xbd, buffer[1]);
+
+    // Seeking way beyond EOS is allowed, doesn't wrap, and read fails.
+    memset(buffer, 0xbd, sizeof(buffer));
+    EXPECT_TRUE(opj_seek_from_memory(
+        std::numeric_limits<OPJ_OFF_T>::max(), &dd));
+    EXPECT_EQ(kReadError, opj_read_from_memory(buffer, 1, &dd));
+    EXPECT_EQ(0xbd, buffer[0]);
+}
diff --git a/pdfium.gyp b/pdfium.gyp
index b6e68aa377..78b334df90 100644
--- a/pdfium.gyp
+++ b/pdfium.gyp
@@ -921,6 +921,7 @@
       ],
       'sources': [
         'core/src/fpdfapi/fpdf_parser/fpdf_parser_decode_unittest.cpp',
+        'core/src/fxcodec/codec/fx_codec_jpx_unittest.cpp',
         'core/src/fxcrt/fx_basic_bstring_unittest.cpp',
         'core/src/fxcrt/fx_basic_wstring_unittest.cpp',
         'testing/fx_string_testhelpers.h',