search

alkacon / opencms-core

The Java open source content management system by Alkacon Software
http://www.opencms.org
GNU Lesser General Public License v2.1
526 stars 576 forks source link

/pdflink for not existing file throws null-error instead of file-not-found #643

Closed patric-dosch-vi closed 5 years ago

patric-dosch-vi commented 5 years ago

hey opencms team,

we use the pdf-generation from OpenCms, so far so good. But when the content is deleted (or just enter a fake url), the /pdflink ends up in 500 instead of a 404, as I would rather expect.

I think in org.opencms.pdftools.CmsPdfLink#CmsPdfLink(org.opencms.file.CmsObject, java.lang.String) CmsUUID id = cms.readIdForUrlName(detailName); do not respect, that the id could be null.

Then the cms.readResource throws “org.opencms.file.CmsVfsException: Error reading resource with the ID null.” and do nothing meaningful with it. That’s bubble up to org.opencms.pdftools.CmsPdfResourceHandler#initResource and throws the exception (because it’s not a filenotfound).

Best, Patric

patric-dosch-vi commented 5 years ago

hello everyone, hello @tHerrmann, the topic generates more and more attention with us, because in the Google Console more and more 500er of this kind appear and unfortunately can not disappear any more.

I think the behavior is a bug and should be fixed. At the moment I'm still missing a clue how exactly. Do you have a clue? Maybe I can contribute something to the solution.

Best, Patric

tHerrmann commented 5 years ago

Hi Patric, the commit above should fix the issue. Please let me know, if it is not working for you. Reards, Tobias

patric-dosch-vi commented 5 years ago

hey @tHerrmann, thank you very much! I also had the fix in the CmsPdfLink, unfortunately I overlooked that the exception has to be handled in the CmsPdfResourceHandler, I suspected that it happens in the core.

Best, Patric

gWestenberger commented 5 years ago

Fixed in branch_11_0_x.