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

Move PDF generation into a separate module #31

Closed
chillu opened this issue Jan 15, 2018 · 5 comments
Closed

Move PDF generation into a separate module #31

chillu opened this issue Jan 15, 2018 · 5 comments

Comments

@chillu
Copy link
Member

chillu commented Jan 15, 2018

The majority of BasePage and BasePageController deal with generating PDFs. That's becoming a less common use case on the web, given that PDF is generally not considered an accessible format for government websites, and because browsers have that built in now (even on Edge).

Even if you don't use this feature on your website, the downloadpdf endpoint is accessible on every page - which increases the DDoS attack surface of every CWP site. It also increases the amount of code a SilverStripe dev needs to review and understand when coming into a CWP project.

Move this feature into a separate module (cwp/pdfexport?). I'd suggest to make it opt-in, and document an upgrade path. This will be an API change (method subclassing on Page no longer works). CWP 2.0 is an opportunity to do that.

You could argue that CWP no longer needs to provide this feature, and hence it shouldn't be a supported module within CWP in the first place (reducing our maintenance burden, e.g. with https://github.com/silverstripe/cwp/issues/3) - but that's a wider discussion that might not be resolved in the CWP 2.0 release timeline.

/cc @tractorcow @robbieaverill

@tractorcow
Copy link

This is a smaller part of the "refactor basepage away forever" that I'd like in cwp. How much of this can we do in 2.0 @robbieaverill ?

@robbieaverill
Copy link
Contributor

Yeah why not :-D

@rupachup rupachup modified the milestones: Sprint 4, Sprint 5 Jan 23, 2018
@robbieaverill robbieaverill removed their assignment Jan 23, 2018
@NightJar
Copy link
Contributor

By the by, Edge is even now the default PDF viewer in Windows.

@chillu
Copy link
Member Author

chillu commented Jan 24, 2018

🎉

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

No branches or pull requests

5 participants