MyFaces Tomahawk
  1. MyFaces Tomahawk
  2. TOMAHAWK-1389

TableSuggestAjax - security popup in IE7 when using SSL

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.8
    • Fix Version/s: 1.1.11
    • Component/s: Alias Bean
    • Labels:
      None
    • Environment:
      myfaces 1.2.5
      tomahawk 1.1.8
      sandbox 1.1.7 snapshot
      facelets 1.1.14
      tomcat 6.16
      Apache mod_jk 2 (issue also comes up without mod_jk)

      Description

      A security popup comes up when using TableSuggestAjax in Internet Explorer 7 (IE7) when using SSL / HTTPS with the message:
      Warning: This page contains secure and non secure items ...
      Warnung: Diese Seite enthält sichere und nicht sichere Objekte ...

      TableSuggestAjax works in IE6, Mozilla, Safari and Google Chrome - just IE7 has this security-issue.

      what is going on in TableSuggestAjax component? Is there a possibility to fix this (with or without code change)?
      (telling IE7 not to report this errors (in ie-preferences) is not meant as fix)

      thank you in advance!

      1. dojo_patched.js
        261 kB
        Gerd Schaffer
      2. TOMAHAWK-1389.patch
        3 kB
        Leonardo Uribe
      3. dojo.js
        262 kB
        Werner Punz

        Activity

        Hide
        Werner Punz added a comment -

        Ok i integrated the patches, but they had slight errors thanks to not taking into consideration that outerHTML is a read only attribute on non ie browsers, please test the patch before I commit it.

        Show
        Werner Punz added a comment - Ok i integrated the patches, but they had slight errors thanks to not taking into consideration that outerHTML is a read only attribute on non ie browsers, please test the patch before I commit it.
        Hide
        Werner Punz added a comment -

        Ok after a customer induced hiatus i am on the bug again, my main problem is now that I cannot reproduce the bug here with a local ssl setting. (Probably due to the self rolled key), nevertheless I will roll the patch fixes in manually and then have one of the people affected testing it.

        Show
        Werner Punz added a comment - Ok after a customer induced hiatus i am on the bug again, my main problem is now that I cannot reproduce the bug here with a local ssl setting. (Probably due to the self rolled key), nevertheless I will roll the patch fixes in manually and then have one of the people affected testing it.
        Hide
        Werner Punz added a comment -

        ok as it seems the, issue is in another codepart and as it seems the outerHTML = "" seems to resolve it, now the problem with outerHTML = "" simply is this approach of clearing the dom tree is not really well working either. I will merge the delete dom code from myfaces in which tackles the problem in a much cleaner way or i will only trigger outerHTML = "" for old ie browsers.

        Either way I have to reopen the issue.

        Show
        Werner Punz added a comment - ok as it seems the, issue is in another codepart and as it seems the outerHTML = "" seems to resolve it, now the problem with outerHTML = "" simply is this approach of clearing the dom tree is not really well working either. I will merge the delete dom code from myfaces in which tackles the problem in a much cleaner way or i will only trigger outerHTML = "" for old ie browsers. Either way I have to reopen the issue.
        Hide
        Werner Punz added a comment - - edited

        Ok after trying to reproduce the bug on ie6 and ie8 ie7 mode which of course failed, I have revisited the patch which
        apparently fixes the issue for the user. Besides the wrong outerHTML which is another issue (probably
        the deleteNode from the myFaces 2 core JS should be integrated which works well over all browsers)
        the only line in the patch which indicates a pointer to a fix for this issue is:

        -div.style.backgroundImage="url(\""this.imgPath"/tab_close.gif\")";

        which is part of an accessibility check which is non critical given tomahawks nature. We
        probably can safely remove this part.
        I will fix this issue give the code a quick test and then set the bug to resolved.

        Show
        Werner Punz added a comment - - edited Ok after trying to reproduce the bug on ie6 and ie8 ie7 mode which of course failed, I have revisited the patch which apparently fixes the issue for the user. Besides the wrong outerHTML which is another issue (probably the deleteNode from the myFaces 2 core JS should be integrated which works well over all browsers) the only line in the patch which indicates a pointer to a fix for this issue is: -div.style.backgroundImage="url(\"" this.imgPath "/tab_close.gif\")"; which is part of an accessibility check which is non critical given tomahawks nature. We probably can safely remove this part. I will fix this issue give the code a quick test and then set the bug to resolved.
        Hide
        Werner Punz added a comment -

        Hello I have checked the patch, dont think this patch really fixes anything, it just replaces a load of node.parentNode.removeElement(node) calls with node.outerHTML = "" calls, which is problematic because outerHTML as writable attribute only works on IE and apparently only up until 7 and 8 Quirksmode I guess.

        But I will give the file a testrun, but I assume we have to work on the resource loading itself here, or how the resources are referenced, I doubt the patch does anything to fix anything here.

        Show
        Werner Punz added a comment - Hello I have checked the patch, dont think this patch really fixes anything, it just replaces a load of node.parentNode.removeElement(node) calls with node.outerHTML = "" calls, which is problematic because outerHTML as writable attribute only works on IE and apparently only up until 7 and 8 Quirksmode I guess. But I will give the file a testrun, but I assume we have to work on the resource loading itself here, or how the resources are referenced, I doubt the patch does anything to fix anything here.
        Hide
        Werner Punz added a comment -

        I am checking out the dojo fixes and patching them in, sorry for the delay, it has been a long time

        Show
        Werner Punz added a comment - I am checking out the dojo fixes and patching them in, sorry for the delay, it has been a long time
        Hide
        Leonardo Uribe added a comment -

        Attached the solution proposed by Gerd Schaffer in patch format.

        Show
        Leonardo Uribe added a comment - Attached the solution proposed by Gerd Schaffer in patch format.
        Hide
        Gerd Schaffer added a comment -

        Concerning to Gerald this is a dojo issue - the dojo files included in tomahawk are rather old and not in current version.
        I got this patched dojo.js file from Manfred - just replace the dojo.js with the attached file in the tomahawk-jar file.
        with this patched file it works.

        Maybe somebody can commit this to the next builds in tomahawk so that this issue is solved also for other users. Thank you.

        regards, Gerd

        Show
        Gerd Schaffer added a comment - Concerning to Gerald this is a dojo issue - the dojo files included in tomahawk are rather old and not in current version. I got this patched dojo.js file from Manfred - just replace the dojo.js with the attached file in the tomahawk-jar file. with this patched file it works. Maybe somebody can commit this to the next builds in tomahawk so that this issue is solved also for other users. Thank you. regards, Gerd
        Hide
        Gerd Schaffer added a comment -

        Hi Simon,
        Thanks for your comments.
        Yes - thats right - IE7 behaves that way.
        The TableSuggestAjax component creates vast amount of javascript.
        The source is not viewable that way - no http:// parts are there in the generated html code (except the doctype definition on top: <?xml version="1.0" encoding="iso-8859-1" ?><!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd" ><html xmlns="http://www.w3.org/1999/xhtml">)

        I think something must be in the java-script part of this component (all other pages - where this component is not used work very well!)
        We currently accept http and https connections - if we just accept https connections (what we plan to do asap) this component will stop working for sure.
        The perfect way should be, that the component uses the same connection schema (http or https) as the main page (as you described).

        Maybe you or someone else can help?
        Thank you in advance!!

        regards, Gerd

        Show
        Gerd Schaffer added a comment - Hi Simon, Thanks for your comments. Yes - thats right - IE7 behaves that way. The TableSuggestAjax component creates vast amount of javascript. The source is not viewable that way - no http:// parts are there in the generated html code (except the doctype definition on top: <?xml version="1.0" encoding="iso-8859-1" ?><!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd" ><html xmlns="http://www.w3.org/1999/xhtml">) I think something must be in the java-script part of this component (all other pages - where this component is not used work very well!) We currently accept http and https connections - if we just accept https connections (what we plan to do asap) this component will stop working for sure. The perfect way should be, that the component uses the same connection schema (http or https) as the main page (as you described). Maybe you or someone else can help? Thank you in advance!! regards, Gerd
        Hide
        Simon Kitching added a comment -

        IE7 has a paranoid security check: if the main page is loaded via an https url, then it complains if any resource loaded by the page (javascript, css, images) is loaded by a normal http url. This check really is too strict; it catches almost no real bugs, but complains about perfectly sane pages. Well, I suppose it might prevent "man in the middle" attacks that inject evil javascript. But in practice it causes far more pain than benefit.

        Other browsers have more sense, and don't bother to apply this check. However given the number of IE7 installations, it is (sigh) reasonable to apply a workaround for this.

        The fix is to ensure that when JSF components write references into the generated page (javascript, css, images, etc) the generated URL always uses the same scheme as the "main" page (ie https when the main page is https).

        In this case, it means a code-chane to the TableSuggestAjax component.

        Gerd, please "view source" on the problem page and report any urls in the page that are using "http://". These are the ones that will need to be fixed.

        Show
        Simon Kitching added a comment - IE7 has a paranoid security check: if the main page is loaded via an https url, then it complains if any resource loaded by the page (javascript, css, images) is loaded by a normal http url. This check really is too strict; it catches almost no real bugs, but complains about perfectly sane pages. Well, I suppose it might prevent "man in the middle" attacks that inject evil javascript. But in practice it causes far more pain than benefit. Other browsers have more sense, and don't bother to apply this check. However given the number of IE7 installations, it is (sigh) reasonable to apply a workaround for this. The fix is to ensure that when JSF components write references into the generated page (javascript, css, images, etc) the generated URL always uses the same scheme as the "main" page (ie https when the main page is https). In this case, it means a code-chane to the TableSuggestAjax component. Gerd, please "view source" on the problem page and report any urls in the page that are using "http://". These are the ones that will need to be fixed.

          People

          • Assignee:
            Werner Punz
            Reporter:
            Gerd Schaffer
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development