Skip to content

fix(PDFWriter): prevent segfault on double EndPDF calls#328

Open
SergeySD wants to merge 1 commit into
galkahana:mainfrom
SergeySD:fix/double-endPDF-segfault
Open

fix(PDFWriter): prevent segfault on double EndPDF calls#328
SergeySD wants to merge 1 commit into
galkahana:mainfrom
SergeySD:fix/double-endPDF-segfault

Conversation

@SergeySD

Copy link
Copy Markdown

Summary

  • Add mIsStarted flag to PDFWriter class to track PDF lifecycle state
  • Prevent crashes when EndPDF() or EndPDFForStream() are called multiple times
  • Return eFailure gracefully instead of causing undefined behavior/segfault

Problem

Calling EndPDF() twice on the same PDFWriter instance causes a segmentation fault because:

  1. EndPDF() calls mDocumentContext.FinalizeNewPDF() which operates on already cleaned-up resources
  2. EndPDF() calls mOutputFile.CloseFile() on already closed file handles
  3. No internal state protection existed to prevent double calls

This affects wrapper code (Node.js, Python, etc.) where:

  • User code calls close/end method
  • Destructor also calls cleanup
  • Error handling paths may call EndPDF multiple times

Solution

Added bool mIsStarted member variable:

  • Set to false in constructor and Reset()
  • Set to true in StartPDF(), StartPDFForStream(), ModifyPDF(), ModifyPDFForStream(), ContinuePDF(), ContinuePDFForStream()
  • Checked at start of EndPDF() and EndPDFForStream() - returns eFailure if not started

Test Plan

  • Added DoubleEndPDFTest.cpp with 4 test cases:
    • Double EndPDF() on file output returns eFailure
    • Double EndPDFForStream() on stream output returns eFailure
    • Reuse after Reset() works correctly
    • EndPDF() without StartPDF() returns eFailure
  • All 83 existing tests pass
  • Test formatted to match project coding style

Add mIsStarted flag to track PDF lifecycle state, preventing crashes when EndPDF or EndPDFForStream are called multiple times. Includes comprehensive test coverage for double-end scenarios and edge cases.
@julianhille

Copy link
Copy Markdown

i would have done it the other way around like "isEnded" instead of "isStarted" but this is the correct fix for double calls to end keeping a state.

@galkahana

Copy link
Copy Markdown
Owner

i don't really see it as the library role to make sure you call endpdf just once. just don't.
if you find it hard to make sure you call just once or at least once, wrap the pdfwriter instance with something to manage the context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants