LibrePDF / OpenPDF

OpenPDF is a free Java library for creating and editing PDF files, with a LGPL and MPL open source license. OpenPDF is based on a fork of iText. We welcome contributions from other developers. Please feel free to submit pull-requests and bugreports to this GitHub repository.
Other
3.64k stars 598 forks source link

Bouncy Castle is not optional #65

Closed drivron closed 6 years ago

drivron commented 6 years ago

A simple test which only instantiate a PdfReader on a empty PDF requires Bouncy Castle in the classpath. The pom declare it optional in the manifest. This problem does not happen with the iText version at ymasory/iText-4.2.0.

drivron commented 6 years ago

My test and the result :

import com.lowagie.text.pdf.PdfReader;

public class Test {
    public static void main(String[] args) throws Exception {
        PdfReader reader = new PdfReader(args[0]);
        reader.close();
    }
}

$ javac -cp openpdf-1.0.6-SNAPSHOT.jar Test.java $ java -cp openpdf-1.0.6-SNAPSHOT.jar:. Testblank.pdf

Exception in thread "main" java.lang.NoClassDefFoundError: org/bouncycastle/cms/Recipient
    at Test.main(Test.java:5)
Caused by: java.lang.ClassNotFoundException: org.bouncycastle.cms.Recipient
    at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
    at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:335)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
    ... 1 more
riccardo-noviello commented 6 years ago

Hello, I haven't found any exception, I set up a project in Maven with openpdf-1.0.6-SNAPSHOT as a dependency

  <dependencies>

     <dependency>
        <groupId>com.github.librepdf</groupId>
        <artifactId>openpdf</artifactId>
        <version>1.0.6-SNAPSHOT</version> <!-- I cloned this repo on the master branch and built the snapshot locally -->
    </dependency>

    <dependency>
      <groupId>junit</groupId>
      <artifactId>junit</artifactId>
      <version>4.12</version>
      <scope>test</scope>
    </dependency>
  </dependencies>

and run the following test

    @Test
    public void testPdfReader() {
        try {
            ByteArrayOutputStream os = new ByteArrayOutputStream();
            Document document = new Document();
            PdfWriter writer = PdfWriter.getInstance(document, os);
            document.open();
            document.add(new Paragraph("Hello World!"));
            document.close();
            writer.close();
            PdfReader reader = new PdfReader(os.toByteArray());
            reader.close();
        } catch (Exception e) {
            fail(e.toString());
        }
    }

This test passes. It could be something relevant to the PDF you are passing as an argument. Although coming back to your first point "Bouncy Castle is not optional", I'd say no, it isn't as the README.md states it is Required.

asturio commented 6 years ago

Hi @rik86, I just checked the pom.xml. I think there is than an error here:

        <plugin>
            <groupId>org.apache.felix</groupId>
            <artifactId>maven-bundle-plugin</artifactId>
            <configuration>
                <!-- unpack bundle, so humans can see the manifest without unpacking the jar -->
                <unpackBundle>true</unpackBundle>
                <instructions>
                    <!-- All com.lowagie.text.* packages are 'public' -->
                    <Export-Package>com.lowagie.text.*;version="${project.version}"</Export-Package>
                    <!-- Declare the Bouncycastle dependencies as optional -->
                    <Import-Package>org.bouncycastle.*;resolution:=optional,!com.lowagie.*,*</Import-Package>
                    <!--                                        UP HERE -->
                </instructions>
            </configuration>
        </plugin>
asturio commented 6 years ago

I don't know the reason for this. Any clue?

riccardo-noviello commented 6 years ago

I think this maven plug in is well old, from a quick read it seems to be for maven 2. I'd say to remove it and make bouncy castle a normal dependency. In maven 3 clashing dependencies can be excluded anyway... also the repository mentioned in the first comment: ymasory/iText-4.2.0 it's an ant build, so it's different

drivron commented 6 years ago

BouncyCastle is optional in iText at execution time until there is an attempt to read or write encrypted PDF, which is far to be the most frequent use case. So this line in the manifest was correct. Also, BC is now declared optional in the iText 5 pom.xml. This constraint prevents us to migrate from iText to OpenPDF.

asturio commented 6 years ago

@rivron, so you mean OpenPDF should keep the optional plugin setup? So the generated jar has this optional flag set?

asturio commented 6 years ago

@rivron, @rik86 There is a PR #68. Could this fix this issue?

drivron commented 6 years ago

The problem is independent from the pom or the manifest. It can only be reproduced with a custom classpath which does not include bouncy castle, i.e outside maven. I have attached a zip which runs the test from @rik86 with 3 different jars:

The test passes with the first and last jar.

This line (and the commented part // OJO...) is one of the difference with iText 4.2.0 and the use of bc1.58. I do not understand why this forces the use of bc, especially when the execution does not reached this line... the method readDecryptedDocObj stops way before because the pdf is not encrypted.

One solution might be to isolate readDecryptedDocObj in a specific class.

asturio commented 6 years ago

Hi @rivron, @rik86, @rtfarte,

I just made some changes in the PdfReader, now your test code is working. Take a look at #68

drivron commented 6 years ago

I have tested it. Thanks a lot @asturio !

asturio commented 6 years ago

If there is no objection, I'll be merging it till the WE.

BernhardPosselt commented 6 years ago

Can we get a release?

keesvandieren commented 6 years ago

Bouncy Castle may give problems on some Application Servers, a release would be great :-)

gexclaude commented 6 years ago

I would appreciate a release too :)

gexclaude commented 6 years ago

As far as I see, release 1.1.0 contains the bouncy castle optionality fix - only had a quick look at the source, and will test asap.

ghost commented 6 years ago

This seems to be fixed now? Please reopen if you still have this problem.

ghost commented 6 years ago

Please try OpenPDF 1.1.0 and report if this has been fixed: https://github.com/LibrePDF/OpenPDF/releases/tag/1.1.0

gexclaude commented 6 years ago

In our case we can now generate the PDFs without bouncycastle - therefore I consider this issue fixed.