IvyDE
  1. IvyDE
  2. IVYDE-329

Disable DTD external fetching for matching ivy/ivy

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.2.0.final
    • Component/s: None
    • Labels:
    • Environment:

      Ivy 2.4.0.alpha/IvyDE 2.2.0.beta2

      Description

      Our team uses Ivy/IvyDE, and noticed Eclipse hanging today/yesterday, to the point of being unusable.

      I tracked it down to Eclipse asking IvyDE "is this your file?", which IvyDE's IvySettingsContentDescriber used XMLHelper.parse to answer, but then Xerces hung while trying to load the XML file's DTD.

      So, the problem was the DTD being unavailable, but it seems like XMLHelper should turn this off, especially if the schema parameter is null, and it's in non-validating mode.

      The attached patch turns off external DTD fetching when XMLHelper is already in non-validating mode.

      This avoids the wire call, which speeds up the XMLHelper.parse by at least 100%, ~250-300ms when fetching the DTD, to ~100-150ms when not. (And this is the happy case, in the worst case, waiting for the timeout if the DTD is unavailable, takes ~20s).

      1. optionalExternalDtds-ivyde.diff
        2 kB
        Stephen Haberman
      2. optionalExternalDtds.diff
        3 kB
        Stephen Haberman
      3. ivyde-xml-dtd-hung.txt
        5 kB
        Stephen Haberman

        Activity

        Hide
        Nicolas Lalevée added a comment -

        Last patches applied on trunk of Ivy and IvyDE.
        I just had to make a clone of XMLParser so that IvyDE doesn't require the lastest version of Ivy.

        Thank you very much for your contribution.

        Show
        Nicolas Lalevée added a comment - Last patches applied on trunk of Ivy and IvyDE. I just had to make a clone of XMLParser so that IvyDE doesn't require the lastest version of Ivy. Thank you very much for your contribution.
        Hide
        Stephen Haberman added a comment -

        Hi. After 3-4 months, this happened again to me today, and was quite frustrating--the Eclipse UI thread blocks and I had to restart Eclipse on a regular basis.

        Based on Maarten's feedback, I revamped my patch to leave the existing functionality as-is for everything except for IvyDE's IvySettingsContentDescriber, which disables looking up external DTDs.

        This should be fine, as IvySettingsContentDescriber only reads an XML file looking for a root "ivysettings" element. I also did the same thing for IvyFileContentDescriber.

        Sorry for having multiple patches uploaded; I'll see if I can delete the old ones.

        Show
        Stephen Haberman added a comment - Hi. After 3-4 months, this happened again to me today, and was quite frustrating--the Eclipse UI thread blocks and I had to restart Eclipse on a regular basis. Based on Maarten's feedback, I revamped my patch to leave the existing functionality as-is for everything except for IvyDE's IvySettingsContentDescriber, which disables looking up external DTDs. This should be fine, as IvySettingsContentDescriber only reads an XML file looking for a root "ivysettings" element. I also did the same thing for IvyFileContentDescriber. Sorry for having multiple patches uploaded; I'll see if I can delete the old ones.
        Hide
        Stephen Haberman added a comment -

        Only disable external DTDs for IvyDE's IvySettingsContentDescriber.

        Show
        Stephen Haberman added a comment - Only disable external DTDs for IvyDE's IvySettingsContentDescriber.
        Hide
        Leon Ephorus added a comment -

        We also encountered this problem. I rolled back to the following (released) versions for which the problems do not occur:

        Apache Ivy 2.3.0.cr2_20121105223351
        Apache Ivy Ant Tasks 2.3.0.cr2_20121105223351
        Apache IvyDE 2.2.0.beta1-201203282058-RELEASE
        Apache IvyDE Resolve Visualizer 2.2.0.beta1-201203282058-RELEASE
        
        Show
        Leon Ephorus added a comment - We also encountered this problem. I rolled back to the following (released) versions for which the problems do not occur: Apache Ivy 2.3.0.cr2_20121105223351 Apache Ivy Ant Tasks 2.3.0.cr2_20121105223351 Apache IvyDE 2.2.0.beta1-201203282058-RELEASE Apache IvyDE Resolve Visualizer 2.2.0.beta1-201203282058-RELEASE
        Hide
        Maarten Coene added a comment -

        I've moved this issue to the IvyDE project because I think if we fix the issue it should be done in the IvyDE codebase.

        Show
        Maarten Coene added a comment - I've moved this issue to the IvyDE project because I think if we fix the issue it should be done in the IvyDE codebase.
        Hide
        Stephen Haberman added a comment -

        Hi Maarten, thanks for taking the time to test my patch--I apologize for not finding this out before asking you take time to look at it.

        I have been running a patched version of Ivy with my proposed fix in my local Eclipse, and thanks so far work, so I was lulled into thinking the semantics where the same. Obviously it does not, so I'll look some more in to this.

        What do you think of adding another arg to XMLHelper.parse, called loadExternalDtds, which defaults=true, but can be set false?

        The reason is that IvyDE's IvySettingsContentDescriptor, AFAICT, does not need external DTDs to work, because it uses a custom default handler to just look for the root element of "ivysettings" and then stops.

        Given how often Eclipse asks IvyDE this question (once per XML file in the project, on a regular basis, AFAICT), it seems useful to allow IvySettingsContentDescriptor to opt out of DTD loading.

        I can work up a patch that does this, if you approve. Of if you have other suggestions, let me know and I can try those out too.

        Thanks!

        Show
        Stephen Haberman added a comment - Hi Maarten, thanks for taking the time to test my patch--I apologize for not finding this out before asking you take time to look at it. I have been running a patched version of Ivy with my proposed fix in my local Eclipse, and thanks so far work, so I was lulled into thinking the semantics where the same. Obviously it does not, so I'll look some more in to this. What do you think of adding another arg to XMLHelper.parse, called loadExternalDtds, which defaults=true, but can be set false? The reason is that IvyDE's IvySettingsContentDescriptor, AFAICT, does not need external DTDs to work, because it uses a custom default handler to just look for the root element of "ivysettings" and then stops. Given how often Eclipse asks IvyDE this question (once per XML file in the project, on a regular basis, AFAICT), it seems useful to allow IvySettingsContentDescriptor to opt out of DTD loading. I can work up a patch that does this, if you approve. Of if you have other suggestions, let me know and I can try those out too. Thanks!
        Hide
        Maarten Coene added a comment -

        I also did a quick test

        for instance, consider this xml:

        <?xml version=\"1.0\" encoding=\"UTF-8\"?>
        <!DOCTYPE doc SYSTEM \"Test.dtd\">
        <doc>a &foo;</doc>
        

        and this Test.dtd:

        <!ENTITY foo "foo">
        

        Without the patch, the SAX handler receives 'a foo' as content of the <doc>-element.
        When I apply your patch, the SAX handler receives 'a' as content. So it seems the &foo; entity is ignored.

        Show
        Maarten Coene added a comment - I also did a quick test for instance, consider this xml: <?xml version=\ "1.0\" encoding=\ "UTF-8\" ?> <!DOCTYPE doc SYSTEM \ "Test.dtd\" > <doc> a &foo; </doc> and this Test.dtd: <!ENTITY foo "foo" > Without the patch, the SAX handler receives 'a foo' as content of the <doc>-element. When I apply your patch, the SAX handler receives 'a' as content. So it seems the &foo; entity is ignored.
        Hide
        Stephen Haberman added a comment -

        I ran a test program against an XML file with entities in it, both valid and invalid entities, and the updated XMLHelper with external-dtd-fetching=false doesn't seem to affect anything.

        Show
        Stephen Haberman added a comment - I ran a test program against an XML file with entities in it, both valid and invalid entities, and the updated XMLHelper with external-dtd-fetching=false doesn't seem to affect anything.
        Hide
        Stephen Haberman added a comment -

        Hm...good point; I will try that and let you know.

        Show
        Stephen Haberman added a comment - Hm...good point; I will try that and let you know.
        Hide
        Maarten Coene added a comment -

        I'm wondering what would happen if the XML file contains entities defined in an external DTD when you disable external DTD fetching?

        Show
        Maarten Coene added a comment - I'm wondering what would happen if the XML file contains entities defined in an external DTD when you disable external DTD fetching?
        Hide
        Stephen Haberman added a comment -

        Fix to disable external DTD fetching.

        Show
        Stephen Haberman added a comment - Fix to disable external DTD fetching.
        Hide
        Stephen Haberman added a comment -

        Stack trace of IvyDE hung on XMLHelper.parse.

        Show
        Stephen Haberman added a comment - Stack trace of IvyDE hung on XMLHelper.parse.

          People

          • Assignee:
            Nicolas Lalevée
            Reporter:
            Stephen Haberman
          • Votes:
            1 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development