Age | Commit message (Collapse) | Author |
|
A suitably corrupted file can cause the parser(s) to repeatedly re-read
sections of the file at increasing parser recursion depth until the
stack is exhausted. There is supposed to be a check for this based upon
the parser "level", but not all call paths pass or update the level as
required.
Much as I hate per-class statics, this introduces one to track the depth
so that the check is enforced no matter how screwy the call path might be
that leads the parser to re-enter itself. This is more palatable than trying
to find all these paths and fix them. We know this is OK since there is
only one thread in here modifying the static.
BUG=451830
R=thestig@chromium.org
Review URL: https://codereview.chromium.org/875263002
|
|
The embeddertests binary should have source files ending in
_embeddertest, not _unittest.
TBR=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/871093004
|
|
We are making checks in the incorrect order. Also adds two test
cases, one for the this crash, and another for the original issue
that motivated the patch.
Original Patch by Bo at https://codereview.chromium.org/866003003/
BUG=450871
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/872563002
|
|
Currently, no callers go through the Interface, which makes having a
separate interface class kind of pointless. After converting callers
away from using the CPDF_DataAvail concrete class, it can be moved
from the header to the .cpp file.
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/873523002
|
|
This also adds a fpdfdoc_embeddertest.cpp to keep the test
file name matching with the API call under test.
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/812933004
|
|
Follow-on work from patch at https://codereview.chromium.org/845643008. This
incorporates that patch, plus adds tests for it and similar conditions. A few
changes are introduced to correct expected behaviour.
This also incoprorates Deepak's fix in https://codereview.chromium.org/845643008/
This incorporates a formerly unlanded tool to generate small valid PDF
files from template input, which is needed to cover all the error cases
here. See https://codereview.chromium.org/791993006/
BUG=450133
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/837723009
|
|
Need to have return value -1 indicating insufficient buffer.
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/862163002
|
|
Previously, UTF16LE_Encode take an optional flag to indicate
if the returned byte string has trailing zeros. In fact, no where
needs the flag to be false. So just get rid of it so callers won't
misuse.
The bug is found by https://codereview.chromium.org/837723009
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/860973002
|
|
This includes:
- Fix TestLoader lifetime.
- Rename test file to match the equivalent .cpp under test
- Re-organize a few tests to avoid duplicate loading
- add tests for a few additional functions.
R=jam@chromium.org
Review URL: https://codereview.chromium.org/857483005
|
|
Previously when passing a NULL bookmark to FPDFBookmark_GetFirstChild it returned NULL instead of returning the top level bookmark. This change removes the early exit in this case allowing the top level bookmark to be retrieved.
BUG=https://code.google.com/p/pdfium/issues/detail?id=110
R=bo_xu@foxitsoftware.com, tsepez@chromium.org
Review URL: https://codereview.chromium.org/847243005
|
|
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/834703002
|
|
This is done by explicitly adding a virtual dtor to interface classes,
since the cost is small given that there are already virtual functions.
The exceptions are for classes that have a Release() or Delete() method,
in which case it is non-virtual and protected to indicate that the virtual
class is never the deletion point.
BUG=
R=brucedawson@chromium.org, thestig@chromium.org
Review URL: https://codereview.chromium.org/810883005
|
|
R=brucedawson@chromium.org
Review URL: https://codereview.chromium.org/837533003
|
|
g_timeMap is a global variable with a constructor and destructor so it
must be removed.
BUG=441899
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/832703003
|
|
PDFium static initializers must go. Static initializers are prohibited
by the style guide. They have negative consequences including increased
startup time (from pulling in additional code pages) and reduced sharing
of data pages (since the variables can't go in the read-only data
segment).
This change uses a template struct and typed enums to reproduce
JS_CalcHash at run-time. An unsigned long long constant and masking with
0xFFFFFFFF are used to avoid compile errors due to integer overflow of
compile-time constants.
The HashVerify class is used to check the results, necessary since none
of the functions in global.cpp are called when pdfium_test.exe runs.
const_expr would be a much cleaner way to implement this change but it
is not yet widely supported.
On the Windows release build this reduces the code size (.text
virtual size) by 0x240 (576) bytes, the .data section by 0x20 bytes
(for eight unsigned globals), and the .rdata section by 0x20 bytes
(the unneeded string savings, minus the eight unsigned globals now
being there).
BUG=441899
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/792043005
|
|
m_sTimeMap is a global variable with a constructor and destructor, which
is not allowed. This change moves it to a function with a static pointer
so that it is constructed on demand and then leaked, thus avoiding
having startup and shutdown code.
This also fixes a worrisome bug caused by having m_sTimeMap defined in
a header file. Because m_sTimeMap was defined (and marked as static) in
a header file there were fifteen separate copies of it, one for each
source file which included the header file. This could easily lead to
bugs because a timer that was added from one source file would be
invisible to other source files.
Each instance of m_sTimeMap added four entries to the
dump-static-initializers.py report, for a total of sixty, so this fix
significantly cleans up that report.
BUG=441899
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/831903002
|
|
Remove CPDF_Dictionary*() operator in CPDF_Bookmark class.
Unify naming conventions and coding styles.
Change some functions to const.
Change the name of function argument to |pDict| for FPDF_xxx type variable.
This makes the code more clear and gives better variable naming
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/828203002
|
|
When dealing with transparency, the printing procedure will generate a bitmap first,
then draw this bitmap in windows DC.
The format of source bitmap is argb, but the destination bitmap is rgb.
Simply doing memcpy will lose the alpha channel information, so CompositeBitmap function is needed.
BUG=412908
R=vitalybuka@chromium.org
Review URL: https://codereview.chromium.org/826633002
|
|
Doing the type conversion on demand is just as efficient as doing it at
startup time, and makes for more efficient startup.
Also mark g_nan as const, to reduce .data section size and enforce
desired semantics.
BUG=441899
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/788143009
|
|
Follow up on https://codereview.chromium.org/733693003
R=brucedawson@chromium.org, tsepez@chromium.org
Review URL: https://codereview.chromium.org/809993004
|
|
QeTable is a 752 byte array that was defined in a header file. This
caused it to be instantiated by the VC++ compiler 12 times, wasting
8,272 bytes of space in the data segment. Because 'const' implies
'static' this did not cause any duplicate symbol errors.
JSCONST_n*HASH are a set of eight variables that are defined in a header
file. This causes them to be replicated 15 times. The variables
themselves are tiny but they are dynamically initialized and this
dynamic initialization code is replicated 15 times.
When tested on pdfium_test.exe the effect of this change is to:
Reduce the .text (code) segment by 3,616 bytes.
Reduce the .rdata section by 8,656 bytes.
Reduce the total binary file size by 13312 bytes.
These are the worst offenders for pdf.dll as shown in:
https://drive.google.com/open?id=1BvubxoA2SU_2e4T5cq7jHTjc1TlT0qOndpIfX3DMeA8&authuser=0
This will also drastically simplify the list of work to be done
for bug 441899 (getting rid of initializers).
BUG=441988
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/802013002
|
|
-remove parameter from FPDF_InitLibrary
-remove a bunch of ifdefs that are unused
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/801913002
|
|
FX_ArraySize macro.
pdfium has numerous places where the number of elements in an array is
calculated with expressions like:
sizeof(cFormats)/sizeof(FX_LPCWSTR)
This is suboptimal because it is verbose, it is easy to get wrong, and
it cannot be determined through casual inspection whether the code is
correct. It will give incorrect results if cFormats is a pointer instead
of an array and it will give incorrect results if FX_LPCWSTR is not the
type of the array elements.
The FX_WSTRC macro in fx_string.h which I fixed was particularly scary
because it would silently misbehave if passed a pointer.
The FX_ArraySize macro which I have added and started using (taken from
arraysize in v8's macros.h) is easier to use and will always give correct
results. If passed a pointer it will fail to compile.
For this change I only fixed instances of sizeof(FX_LPCWSTR). There
appear to be about 150 other places in the pdfium code that could
benefit from using FX_ArraySize.
R=bo_xu@foxitsoftware.com, tsepez@chromium.org
Review URL: https://codereview.chromium.org/729293003
|
|
Since casts to FX_LPCWSTR have been shown to hide bugs I tried removing
more of them, targeting those places where a cast was used to force a
conversion from CFX_WideString to FX_LPCWSTR, replacing these casts with
calls to the newly added .c_str() function. This revealed two places
where the cast was hiding a bug -- where ->c_str() was required instead!
This removes ~33 FX_LPCWSTR casts and there are ~31 left, many of which
will go away in some future change.
Also includes this change:
Removing unnecessary casts from wchar_t* to wchar_t*, by various names.
Original patch from Bruce Dawson(brucedawson@chromium.org)
R=bo_xu@foxitsoftware.com, tsepez@chromium.org
Review URL: https://codereview.chromium.org/733693003
|
|
Remove casts that merely cast from wchar_t* to wchar_t*. Sometimes the
types or casts are FX_LPCWSTR but the idea is the same. Excess casts
can (and have) hidden bugs so removing these may prevent future problems.
Original patch from Bruce Dawson(brucedawson@chromium.org)
R=bo_xu@foxitsoftware.com, tsepez@chromium.org
Review URL: https://codereview.chromium.org/730993002
|
|
Complementary patch in chromium is in https://codereview.chromium.org/711553003
R=thestig@chromium.org
Review URL: https://codereview.chromium.org/700373006
|
|
BUG=https://code.google.com/p/pdfium/issues/detail?id=78
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/726143002
|
|
Found by VC++'s /analyze. Warning was:
fpdfsdk\src\javascript\js_runtime.cpp(352) : warning C6276:
Cast between semantically different string types: char * to wchar_t *.
Use of invalid string can lead to undefined behavior.
This mismatch has been there as far back as the history goes (to May of this year).
It looks like a real bug to me. However I don't know the implications of this bug and why it would not have been noticed at run-time.
The code has been this way as far back as the git history goes, but that is only to May 2014.
Original patch from Bruce Dawson(brucedawson@chromium.org)
BUG=427616
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/705503004
|
|
found by /analyze on some error paths
Warning from /analyze was:
src\third_party\pdfium\fpdfsdk\include\fsdk_mgr.h(96) : warning C6001: Using uninitialized memory 'fxtime'.
Other error paths can also lead to reading from an uninitialized _FX_SYSTEMTIME object.
Code-gen for the constructor is small enough (four writes of zeroed EAX with VC++, less with gcc) to make putting the constructor in a .cc file unnecessary.
Approval of in-class member initialization would make this fix simpler but that has not quite been approved yet.
BUG=https://code.google.com/p/pdfium/issues/detail?id=70
BUG=427616
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/692533005
|
|
BUG=425129
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/688303003
|
|
BUG=pdfium-52
R=jun_fang@foxitsoftware.com
Review URL: https://codereview.chromium.org/623893003
|
|
BUG=410326
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/594403003
|
|
BUG=none
R=jam@chromium.org
Review URL: https://codereview.chromium.org/581413002
|
|
Security handler revision number is needed to interpret file permission.
BUG=None
R=thestig@chromium.org
Review URL: https://codereview.chromium.org/589813002
|
|
but no its direct object '112 0 object' in the test pdf file. Without checking the validity, it causes a null pointer when trying to get the direct object by an indirect object.
BUG=390781
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/553613003
|
|
app::response().
I also clean up the code while we are here, rewriting a strange switch statement and tidying whitespace.
BUG=406142
R=jun_fang@foxitsoftware.com
Review URL: https://codereview.chromium.org/498773004
|
|
In CPDFSDK_InterForm::SubmitFields, the buffer pointed by m_pBuffer is returned
and released by the caller. However, it will be released again in the destructor.
BUG=401580
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/481733002
|
|
BUG=387969
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/461343003
|
|
BUG=chromium:395832
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/478353002
|
|
Should there be cases where this fails to compile, it indicates a mistake,
either an incorrectly declared overrriden virtual method, or a method that
should be declared non-virtual.
The only issues were with CPDF_CustomAccess::GetBlock(), CPDF_CustomAccess::GetByte(),
and CPDF_CustomAccess::GetFullPath(). These don't appear to be used anywhere,
and are removed. Two members are removed that are no longer needed once those
methods are removed.
R=jam@chromium.org, jun_fang@foxitsoftware.com
Review URL: https://codereview.chromium.org/454983003
|
|
BUG=pdfium:28
R=thakis@chromium.org
Review URL: https://codereview.chromium.org/472563002
|
|
To be complient with PDF reference chapter 7.3.7
BUG=402437
R=vitalybuka@chromium.org
Review URL: https://codereview.chromium.org/469573002
|
|
This has no ill-effect at present, but may be distracting when viewing the file
since it just looks wrong.
R=jun_fang@foxitsoftware.com
Review URL: https://codereview.chromium.org/461933003
|
|
BUG=400662
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/445303002
|
|
BUG=https://code.google.com/p/pdfium/issues/detail?id=35
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/451483003
|
|
Edge closer to the goal of building PDFium with the chromium_code
configuration.
BUG=https://code.google.com/p/pdfium/issues/detail?id=29
R=bo_xu@foxitsoftware.com, thakis@chromium.org
Review URL: https://codereview.chromium.org/441763002
|
|
When newPos == file size, the current block will not be read or Get. If this block is a crucial part of the document (like m_pTrailer), the program will exit with parse error and
the document will not be rendered.
BUG=None
R=jun_fang@foxitsoftware.com
Review URL: https://codereview.chromium.org/440563003
|
|
No intended behavior change.
BUG=pdfium:29
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/436483002
|
|
It's unused, and it caused a warning about CPDFSDK_Widget::ResetAppearance()
failing to override it (since these two unrelated methods had the same name).
No intended behavior change.
BUG=pdfium:29
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/429483004
|
|
No intended behavior change.
BUG=pdfium:29
R=bo_xu@foxitsoftware.com
Review URL: https://codereview.chromium.org/426763003
|