search

eclipse-jdt / eclipse.jdt.debug

Eclipse Public License 2.0
16 stars 46 forks source link

Resolve PLIST entities to zero-length content. #324

Closed nitind closed 11 months ago

nitind commented 11 months ago

https://github.com/eclipse-jdt/eclipse.jdt.debug/issues/323 Not doing this causes the parsing to still attempt to retrieve DTDs.

iloveeclipse commented 11 months ago

@nitind : I'm not familiar with XML parsing at all, could you please give more insights what exactly the patch is doing and why it is safe to "Resolve PLIST entities to zero-length content". I must confess I have no idea what PLIST is...

iloveeclipse commented 11 months ago

/rebase

iloveeclipse commented 11 months ago

Note: the build failures can be ignored, see https://github.com/eclipse-pde/eclipse.pde/issues/782.

@jukzi : since you've recently changed lot of XML related code in the platform, could you please check if similar change should also be applied in other places where we start XML parsing with DocumentBuilder.parse?

nitind commented 11 months ago

It's a preference file for applications on macOS, in this case, content that I think is generated on the fly with no reason to use character or parameter entities. As the code is already written with the intent to not validate the contents of the XML, and there's no reason to expect any internal entities in the XML, it's simplest to drop any such requests instead of relying on parser configuration properties that may be implementation-specific.

Entity resolvers are supposed to resolve an entity into a SAX InputSource, which provides both the resolved location information and the content of what it resolved to. Returning "null" as the InputSource from a different entity resolver results in the current XML parser calling a default entity resolver as a failsafe, which then opens a network connection as it does when you don't explicitly set a resolver at all. For that reason, the proposed solution (as was implemented in WTP for reading JSP tag library and web deployment descriptor files) is to explicitly set an entity resolver that more or less drops such requests on the floor.

jukzi commented 11 months ago

That code uses createDocumentBuilderIgnoringDOCTYPE() i.E. it should ignore Doctype i.e. ignore any DTD. the patch is not wrong though, but i don't see any benefit from it. proof me wrong with a junit

iloveeclipse commented 11 months ago

That code uses createDocumentBuilderIgnoringDOCTYPE() i.E. it should ignore Doctype i.e. ignore any DTD. the patch is not wrong though, but i don't see any benefit from it. proof me wrong with a junit

Have you seen the stack on the ticket?