PDFBox
  1. PDFBox
  2. PDFBOX-712

SecurityHandlersManager May stop the application Server when running PDFParser in a Servlet.

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.1.0
    • Fix Version/s: 1.8.6
    • Component/s: PDModel
    • Labels:
      None

      Description

      When parsing a PDF document within an Application Server, you should never have a code path which call System.exit()
      I am not sure what invokes the Class org.apache.pdfbox.pdmodel.encryption.SecurityHandlersManager() from with the code, so I have not a clue how to fix this.

      I imagine that the best place to notify PDFBox that it is running in an application would be something like this.

      PDDocument.setApplication(true or false);

      I would like to be able to tell the Parser that it is not running as an application so this code is never hit, but I did not see a way to do this.

      catch(Exception e)

      { System.err.println("SecurityHandlersManager strange error with builtin handlers: " + e.getMessage()); System.exit(1); }

      Bug: new org.apache.pdfbox.pdmodel.encryption.SecurityHandlersManager() invokes System.exit(...), which shuts down the entire virtual machine
      Pattern id: DM_EXIT, type: Dm, category: BAD_PRACTICE

      Invoking System.exit shuts down the entire Java virtual machine. This should only been done when it is appropriate. Such calls make it hard or impossible for your code to be invoked by other code. Consider throwing a RuntimeException instead.

        Activity

        Hide
        Peter_Lenahan@ibi.com added a comment -

        I am currently on vacation
        I have no access to e-mail.

        Peter

        Show
        Peter_Lenahan@ibi.com added a comment - I am currently on vacation I have no access to e-mail. Peter
        Hide
        Tilman Hausherr added a comment -

        I agree that a System.exit() call in a servlet is a big no-no, but I am also undecided about what to do. Either swallow the exception and output an error log, or rethrow it. Ideas, anyone?

        Show
        Tilman Hausherr added a comment - I agree that a System.exit() call in a servlet is a big no-no, but I am also undecided about what to do. Either swallow the exception and output an error log, or rethrow it. Ideas, anyone?
        Hide
        Petr Slaby added a comment -

        Letting the exception through seems to be the right thing. The constructor and getInstance() of SecurityHandlersManager can declare BadSecurityHandlerException since the PDDocument expects it anyway. PDFBox 2.0 seems to use runtime exceptions instead. Since an instance of the SecurityHandlerFactory is created in its static part, I assume an ExceptionInInitializerError would be thrown. This is not so nice as it obfuscates the original problem and breaks the complete application in the moment when the SecurityHandlerFactory class is loaded.

        Show
        Petr Slaby added a comment - Letting the exception through seems to be the right thing. The constructor and getInstance() of SecurityHandlersManager can declare BadSecurityHandlerException since the PDDocument expects it anyway. PDFBox 2.0 seems to use runtime exceptions instead. Since an instance of the SecurityHandlerFactory is created in its static part, I assume an ExceptionInInitializerError would be thrown. This is not so nice as it obfuscates the original problem and breaks the complete application in the moment when the SecurityHandlerFactory class is loaded.
        Hide
        John Hewson added a comment - - edited

        In 2.0 the exceptions will never be thrown - they're a quirk of old fashioned Java API design. IllegalStateException can be thrown if the user registers the same provider twice, but that can't happen until after the static initializer has been called, so you're always safe.

        In 1.8 calling System.exit() is obviously horrible, however none of the conditions which trigger an exception are true when calling registerHandler from SecurityHandlersManager(), so the catch block can safely be replaced with:

        catch(BadSecurityHandlerException e)
        {
            // should never happen
            return new RuntimeException(e);
        }
        
        Show
        John Hewson added a comment - - edited In 2.0 the exceptions will never be thrown - they're a quirk of old fashioned Java API design. IllegalStateException can be thrown if the user registers the same provider twice, but that can't happen until after the static initializer has been called, so you're always safe. In 1.8 calling System.exit() is obviously horrible, however none of the conditions which trigger an exception are true when calling registerHandler from SecurityHandlersManager(), so the catch block can safely be replaced with: catch (BadSecurityHandlerException e) { // should never happen return new RuntimeException(e); }
        Hide
        Tilman Hausherr added a comment -

        Fixed in rev 1601746 in the 1.8 branch.

        Show
        Tilman Hausherr added a comment - Fixed in rev 1601746 in the 1.8 branch.

          People

          • Assignee:
            Tilman Hausherr
            Reporter:
            Peter_Lenahan@ibi.com
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development