Skip to content

Set OpenPDF profile active by default in flying-saucer-pdf. #123

Closed
ghost wants to merge 1 commit into
masterfrom
unknown repository
Closed

Set OpenPDF profile active by default in flying-saucer-pdf. #123
ghost wants to merge 1 commit into
masterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Mar 17, 2017

Set OpenPDF profile active by default in flying-saucer-pdf.

The iText profile specifies iText version 2.1.7, which is no longer maintained and vulnerable to CVE-2017-9096 iText XML External Entity Vulnerability.

… profile specifies iText version 2.1.7, which is no longer maintained.
@pbrant
Copy link
Copy Markdown
Member

pbrant commented Mar 20, 2017

Thanks. Would there be a way to publish this as a separate artifact instead of changing the default? For a project already using (old) iText, it might be surprising if another JAR is pulled in.

@ghost
Copy link
Copy Markdown
Author

ghost commented Mar 20, 2017

I was really hoping to encourage people to upgrade from the old unmaintained iText 2.1.7 to a maintained and updated version of OpenPDF. So when the users of Flyingsaucer update to the latest version of Flyingsaucer, then they will also get the latest maintained and updated version of this dependency: which is currently OpenPDF.

OpenPDF has updated the versions of the libraries it depends upon, such as to the latest version of the Bouncy Castle Crypto APIs. There are also many bugfixes in the latest version of OpenPDF. Still, it is compatible.

Users who depend on the old iText version 2.1.7 can still build Flyingsaucer with the itext maven profile. However, that version of iText was released in 2009, so I really think that we should encourage users to update.

As one of the maintainers of OpenPDF, I hope that you will change the default to OpenPDF instead of keeping the very old version as default. This is a change to update to a maintained version of a dependency, which would be in the interest of most users of both libraries.

@ghost
Copy link
Copy Markdown
Author

ghost commented Apr 5, 2017

@pbrant How do you suggest this be solved? Any chance you would accept this change?

@pbrant
Copy link
Copy Markdown
Member

pbrant commented Apr 7, 2017 via email

@ghost
Copy link
Copy Markdown
Author

ghost commented Apr 16, 2017

This seems like a good solution. Could you please make such a separate
flying-saucer-pdf-openpdf artifact release?

@ghost
Copy link
Copy Markdown
Author

ghost commented Nov 16, 2017

iText 2.1.7 is vulnerable to CVE-2017-9096 iText XML External Entity Vulnerability http://seclists.org/bugtraq/2017/Nov/20

I would recommend switching from iText 2.1.7 to OpenPDF 1.0.5, where this vulnerability is fixed.
https://github.com/LibrePDF/OpenPDF/releases/tag/1.0.5

@illumi
Copy link
Copy Markdown

illumi commented Nov 21, 2017

My concern is that projects that have a separate dependency on iText will end up with both on the class path and depending on the situation it may be hit and miss which one actually gets used.

As one of those people, I'd welcome an update to use OpenPDF!

@asturio
Copy link
Copy Markdown

asturio commented Nov 21, 2017

Maybe a separate artifact using openpdf could help people, switch to openpdf. We are also using itext now, and flyingsaucer in the company. If there is a flyingsaucer-openpdf build from maven central, we could also change itext to openpdf. Or else we need to work with lots of exclusions.

But I think you should keep 2 different artifact published.

Just my 2 ¢.

@ghost
Copy link
Copy Markdown
Author

ghost commented Nov 21, 2017

Great idea, @asturio. Perhaps you are the person capable of publishing the flyingsaucer-openpdf release to maven central?

@pbrant
Copy link
Copy Markdown
Member

pbrant commented Nov 21, 2017 via email

@ghost
Copy link
Copy Markdown
Author

ghost commented Nov 22, 2017

Given that iText 2.1.7 has a known, unfixed security vulnerability, it could be argued that continuing to release it is unethical. In my opinion, iText 2.1.7 should be replaced by OpenPDF.

So accepting this pull-request, then updating the OpenPDF version to 1.0.5 in flying-saucer-pdf/pom.xml is all you have to do.

@asturio
Copy link
Copy Markdown

asturio commented Nov 24, 2017

There are some possibilities. I think the best is, if the build are done by the project maintainer, alas @pbrant .

I suggest haven a separate branch here, with everything set to make *-openpdf artifacts. And the development beeing made on the master-branch. Every time a new release is released, the changes from master should also be merged in this fs-openpdf branch, and that branch released. And the only differences between the 2 branches should be, that one uses itext and the other openpdf. And of course the artifact names.

The other option should be to make this through maven profiles, but the tweaking of artifact names is not that straight forward.

I think the 1st. option will cope best and should be easier to implement, even in the CI-Environment.

@pbrant: Would be option 1 ok for you? I could prepare a branch, to be pulled in this new branch here (which should exist before I can create the pull request).

@pbrant
Copy link
Copy Markdown
Member

pbrant commented Nov 24, 2017 via email

@ghost
Copy link
Copy Markdown
Author

ghost commented Dec 21, 2017

@asturio @pbrant Any news on this security issue in Flyingsaucer / iText 2.1.7?

@asturio
Copy link
Copy Markdown

asturio commented Dec 23, 2017

Too much work at work :-). Doing this now.

@ghost
Copy link
Copy Markdown
Author

ghost commented Jan 5, 2018

<!-- https://mvnrepository.com/artifact/org.xhtmlrenderer/flying-saucer-pdf-openpdf -->
<dependency>
    <groupId>org.xhtmlrenderer</groupId>
    <artifactId>flying-saucer-pdf-openpdf</artifactId>
    <version>9.1.11</version>
</dependency>

https://mvnrepository.com/artifact/org.xhtmlrenderer/flying-saucer-pdf-openpdf/9.1.11

@Marco-Sulla
Copy link
Copy Markdown

I would contribute with a little modification. Where is the source code of flying-saucer-pdf-openpdf?

@pbrant
Copy link
Copy Markdown
Member

pbrant commented Mar 24, 2018 via email

@ST-DDT
Copy link
Copy Markdown

ST-DDT commented May 17, 2018

It looks like there is no maven artifact for flying-saucer-pdf-openpdf:9.1.13 in the public repos yet.

@stephane-dereppe
Copy link
Copy Markdown

Could you merge the 9.1.13 tag to the openpdf branch and deploy it to maven central please?

@pbrant
Copy link
Copy Markdown
Member

pbrant commented Jun 19, 2018 via email

@ghost
Copy link
Copy Markdown
Author

ghost commented Jun 19, 2018

@pbrant Can you please update to OpenPDF version 1.0.5 in flying-saucer-pdf-openpdf also?

@ghost
Copy link
Copy Markdown
Author

ghost commented Jun 25, 2018

I can't find flying-saucer-pdf-openpdf 9.1.14 here:
https://mvnrepository.com/artifact/org.xhtmlrenderer/flying-saucer-pdf-openpdf

Are you sure it has been published?

@ghost
Copy link
Copy Markdown
Author

ghost commented Jul 11, 2018

OpenPDF 1.1.0 has been released. https://github.com/LibrePDF/OpenPDF/releases/tag/1.1.0

Can you please update to use that in this project?

@ST-DDT
Copy link
Copy Markdown

ST-DDT commented Jul 11, 2018

9.1.14 is now in the maven repo.

@andreasrosdalw
Copy link
Copy Markdown
Contributor

andreasrosdalw commented Mar 6, 2019

@pbrant Any news on setting OpenPDF as the default?
OpenPDF is the future, not the old iText 1.2.x version.

@GitMensch
Copy link
Copy Markdown

@pbrant with ccfadb6 this PR can be closed, can't it?

@andreasrosdalw
Copy link
Copy Markdown
Contributor

andreasrosdalw commented Apr 30, 2019

This PR could be merged now, not closed. OpenPDF should be the default.

@benjohnde
Copy link
Copy Markdown

benjohnde commented Jun 5, 2019

Any updates on this issue?

@asolntsev
Copy link
Copy Markdown
Member

@benjohnde @andreasrosdal FYI I took over FlyingSaucer project maintenance. We can continue discussion here. I also think that switching to OpenPDF is a good idea.

@andreasrosdalw
Copy link
Copy Markdown
Contributor

@asolntsev Congratulations! Will you please make OpenPDF the default PDF library for FlyingSaucer?

@asolntsev
Copy link
Copy Markdown
Member

@andreasrosdal Yes, it's the plan. I just need some time to investigate the background, the release process etc.

@andreasrosdalw
Copy link
Copy Markdown
Contributor

Further, my advice for making Flying Saucer a success in the future would be to add support for more modern HTML5 syntax, and not throwing exceptions if some HTML element is invalid.

@asolntsev
Copy link
Copy Markdown
Member

@andreasrosdal Thank you for sharing your vision.

Yes, I believe it would be great, BUT who is going to implement it? I rather think I don't have enough resources to implement modern HTML5 syntax... Sounds like a huge work to do.

@andreasrosdalw
Copy link
Copy Markdown
Contributor

andreasrosdalw commented Sep 21, 2023

Yes, I believe it would be great, BUT who is going to implement it?

There are two separate issues: the first is that Flying Saucer throws exceptions whenever it parses a nonvalid xhtml element, such as < br >, it needs the xhtml < br />. I wonder if this can be solved with some simple exception handling?

The other issue is full support for html5. Can it be done, by focusing on one element at a time, focusing on the most important elements first?

Maybe people on Github will create pull requests if there are specific tasks on Github for each task of full html5 support?

Also, cooperation and copying from
https://github.com/danfickle/openhtmltopdf

@andreasrosdalw
Copy link
Copy Markdown
Contributor

@pbrant
Copy link
Copy Markdown
Member

pbrant commented Sep 21, 2023

We've used https://about.validator.nu/htmlparser/ at work for a long time to provide HTML5 syntax support. My assumption has been that a lot of folks do something similar when they embed FS in a larger application.

Supporting CSS Flexbox, Grid, etc. would indeed be quite a lot of work though.

@asolntsev
Copy link
Copy Markdown
Member

@andreasrosdal @pbrant I would mention here: you strongly wish to use OpenPDF by default because you assuming it's newer or better supported?
But I see this assumption is false.

I submitted an important pull request to OpenPDF more than month ago:
LibrePDF/OpenPDF#955

Nobody has merged it. Nobody is even reacting to messages. Seems OpenPDF project is dead. :(

@asturio
Copy link
Copy Markdown

asturio commented Oct 31, 2023 via email

@andreasrosdalw
Copy link
Copy Markdown
Contributor

@asturio If you would like help maintaining OpenPDF, I can help with maintenance again. Please add me as a maintainer again and I can help with reviewing and merging PRs and making a new release.

@andreasrosdalw
Copy link
Copy Markdown
Contributor

LibrePDF/OpenPDF#955 has been merged now. Hopefully we will create a new release of OpenPDF with this included very soon.

@asolntsev
Copy link
Copy Markdown
Member

Sorry for that, I really have very small spare time for maintaining the project.

Please forgive me if my words sounded bad. I didn't want to blame anybody personally. We all need rest and do the open-source projects at spare time.

I just tried to estimate the project overall status.

NB! I believe all mature open-source project should have several maintainers who can merge pull requests and release new versions. Without it, any project sooner or later become a "forgotten child".
@asturio @andreasrosdal I highly recommend to take some actions in this direction.

@andreasrosdalw
Copy link
Copy Markdown
Contributor

@asolntsev You are welcome to contribute to and help maintain OpenPDF also. Thank you.
In particular PDF/A support LibrePDF/OpenPDF#980, FIPS Compliance LibrePDF/OpenPDF#921, improving PNG loading speed LibrePDF/OpenPDF#596, and generally supporting new PDF standards, are some areas we need help.

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.

10 participants