search

qos-ch / reload4j

reload4j is a drop-in replacement for log4j 1.2.17
Apache License 2.0
148 stars 22 forks source link

[#34] Make `setFeature` calls optional #56

Closed ppkarwasz closed 1 year ago

ppkarwasz commented 1 year ago

IMHO a missing DocumentBuilderFactory.setFeature() method or support for disabling external entities should not cause a parsing error, just warnings.

ceki commented 1 year ago

@ppkarwasz While I have great respect for your abilities, I had to revert this change as it potentially opens the code for a vulnerability, however unlikely. In case of doubt, I prefer to err on the side of caution.

I understand that this might seem silly given that the code in question, if it throws an exception, then the underlying SAX implementation will probably not honor the setFeature calls. However, it seems preferable to let the users know about it instead of trying to compensate for it.

ppkarwasz commented 1 year ago

@ceki,

Since you are ultimately responsible for the security of Reload4j, I completely understand your point of view.

This PR was motivated by this question on StackOverflow, although I never asked the user for more details and the user in question never filed a bug report.