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

Resolve PLIST entities to zero-length content. #324

Merged
merged 1 commit into from
Oct 14, 2023

Conversation

nitind
Copy link
Contributor

@nitind nitind commented Oct 13, 2023

#323 Not doing this causes the parsing to still attempt to retrieve DTDs.

@iloveeclipse
Copy link
Member

@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
Copy link
Member

/rebase

@iloveeclipse
Copy link
Member

Note: the build failures can be ignored, see eclipse-pde/eclipse.pde#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
Copy link
Contributor Author

nitind commented Oct 13, 2023

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
Copy link
Contributor

jukzi commented Oct 14, 2023

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
Copy link
Member

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?

@iloveeclipse iloveeclipse merged commit 399fc74 into eclipse-jdt:master Oct 14, 2023
5 checks passed
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.

3 participants