Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for write-protected PDFs #942

Merged
merged 2 commits into from
Apr 13, 2016
Merged

Add support for write-protected PDFs #942

merged 2 commits into from
Apr 13, 2016

Conversation

koppor
Copy link
Member

@koppor koppor commented Mar 11, 2016

This PR shows how support for write-protected PDFs can be added to JabRef.

If we do not want to be a "cryptographic" software, I can just remove the dependency on org.bouncycastle:bcprov-jdk15on:1.54. Then, JabRef will compile, but throw an exception if write-protected PDFs are opened. I can adjust the test cases accordingly.

Crypto Discussion

This fixes #935.

When importing PDFs, which carry a password for editing, both the XMP Metadata parser and the PDFContentImporter can handle this.

Now, the Legion of the Bouncy Castle Java cryptography APIs are part of JabRef, which makes JabRef subject to export restrictions. Bouncy Castle is approved classified under ECCN code 5D002 and approved for export under License Exception TSU. According to https://www.bis.doc.gov/index.php/policy-guidance/encryption/classification, JabRef can now only be exported "to any end-user located or headquartered in a country listed in Supplement No. 3 to Part 740", which is AustriaAustraliaBelgiumBulgariaCanadaCyprusCzech RepublicEstoniaDenmarkFinlandFranceGermanyGreeceHungaryIcelandIrelandItalyJapanLatviaLithuaniaLuxembourgMaltaNetherlandsNew ZealandNorwayPolandPortugalRomaniaSlovakiaSloveniaSpainSwedenSwitzerlandTurkeyUnited Kingdom.

The current code, however, also allows BouncyCastle to be removed and JabRef still compiles. It internally throw a NoClassDefFound Exception, which is handled properly.

I am unsure, how to proceed. Debian also distributes bouncy-castle.

@Siedlerchr
Copy link
Member

From what I understand in this discussion, that the US exporting laws would apply if you distribute the software from an US Server:
http://security.stackexchange.com/questions/7015/us-export-control-on-open-source-libraries

The best solution would be to load it dynamically and not distribute it with JabRef.
Then users can download the library, put them in the folder and use it.

@koppor koppor changed the title Add support for write-protected PDFs [WIP] Add support for write-protected PDFs Mar 12, 2016
@stefan-kolb
Copy link
Member

To be honest I don't think it is worth to make such a hassle just because of a partially working PDF content importer functionality. I mean how often do we really need this? Isn't it acceptable if we need to get the BibTex entry by a DOI or set it manually if the PDF is encrypted? It really just is a problem if we do not already have the BibTEx entry (which is probably often the case) or don't supply a DOI etc. Think we could discuss this in a call if I'm not the only one concernced about such a change.

@koppor
Copy link
Member Author

koppor commented Mar 12, 2016

It's also about XMPUtil, which is really used in the wild. I agree that write-protected PDFs are not the majority of the PDFs in the wild.

With this patch, we could distribute JabRef without BouncyCaslte and users requiring that feature can compile a crypto-aware JabRef for themselves. Linux distributions can decide for themselves whether they want to enable that feature in their binary distributions.

@koppor koppor changed the title [WIP] Add support for write-protected PDFs Add support for write-protected PDFs Mar 13, 2016
try (InputStream is = XMPUtilTest.class.getResourceAsStream("/pdfs/write-protected.pdf")) {
List<BibEntry> bibentries = XMPUtil.readXMP(is);
Assert.assertEquals(1, bibentries.size());
BibtexEntryAssert.assertEquals(this.getClass(), "write-protected.bib", bibentries.get(0));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to define an external bib file here...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed 😇. - I've been in a mode that I read the resulting bib files and not interpret code. The code is ca. 6 lines longer, but we have one file less. Hope, that is OK now. a39f11f#diff-4538b7a42e8c15776dcbb34bca7c25d3R882

 * add BouncyCastle library
 * remove BouncyCastle installation warning string
 * add test file write-protected.pdf
 * better exception name
BibEntry entry = new BibEntry();
entry.setType("misc");
entry.setField("author", "Firstname Lastname");
List<BibEntry> expected = new ArrayList<>(1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Collections.singletonList

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 82b5b7c

@tobiasdiez
Copy link
Member

LGTM 👍

@koppor
Copy link
Member Author

koppor commented Apr 13, 2016

DevCall result: We want to enable reading of write-protected PDFs as they are not that seldom. In the future, JabRef will have better support for PDF text and PDF comments (#139). Thus, this PR enables treating of more PDFs.

@koppor koppor merged commit dce8654 into master Apr 13, 2016
@koppor koppor deleted the cryptedpdfs branch April 13, 2016 06:35
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.

Add support for parsing of encrypted PDFs
4 participants