Uploaded image for project: 'Z-Admin'
  1. Z-Admin
  2. ADM-215

responses to file upload requests are not properly handled by the UI

    Details

    • Type: Bug
    • Status: QA
    • Priority: Normal
    • Resolution: Fixed
    • Affects Version/s: 1.3.0
    • Fix Version/s: 1.4.0
    • Component/s: None
    • Security Level: Public (Everyone)
    • Labels:
      None
    • Severity:
      Normal
    • Impact:
      High
    • Similar issues:
      ADM-239yaffas-core: properly handle file upload requests with query strings
      ADM-206fetchmail: invalid input in the UI can permanently break UI and fetchmail config
      ADM-260User-specific quota settings are not properly verified in the UI
      ADM-264RHEL: multiple search domains in /etc/resolv.conf are not properly shown in the UI
      ADM-289Network > Configuration cannot cope with quotes in CentOS ifcfg-* files

      Description

      zadmin is unable to properly handle responses to file upload requests. This problem currently manifests at at least two occasions:

      This is not a regression, I tested this also on older yaffas versions (v1.1, v1.2.3) which also shows the problem, both in Firefox 16.0.2 and Chromium 21.0.1180.89.

      As far as we remember, "almost the same code" (needs to be verified, especially the YUI2 version) worked in bitkit.

      The problem is related to YUI's response object's responseText, which we use to get the HTML. This does not look correct, it probably should be responseXML (or some subattribute). But such a change needs lots of testing with different browsers.

      The problem exists for file upload requests only, because YUI uses another request method instead (usually it uses AJAX/XHR, but for file uploads YUI uses a hidden iframe).

        Gliffy Diagrams

          Activity

          Hide
          bbhoffmann Christian Hoffmann added a comment - - edited

          Fixed in aa71bfdb53ba2151fc0d65389cf7b31f55fa3f3b in the adm-215 branch (not in devel yet!).

          Our UI response parsing code was wrong, although it worked in most cases. We are using the responseText property of YUI2's async response objects. This apparently works fine for AJAX/XHR requests, but it breaks whenever iframes are used as the request method (YUI2 transparently does that for file uploads). Where "breaks" means: .responseText contains only the text (all HTML stripped) in these cases (as documented...). The correct way to go is to use .responseXML. Sadly, this is not always populated.

          So the fix is: We try to use .responseXML, if available, but fall back to .responseText in all other cases. This seems to work fine on Firefox and Chrome for all types of requests (GET, POST, POST w/ file upload).

          As .responseText was used multiple times in ui.js, I moved this decision logic to its own function, which is now used instead.
          (To avoid code duplication)

          Notes for testing: The fix touches multiple code paths, which are hit on each and every request. Breakage likelihood is very high, therefore. Tests should be done for all types of requests:

          • GET, i.e. some data table
          • POST, e.g. Zarafa -> Configuration -> Save
            • valid input (verify that changes are saved properly)
            • bad input (e.g. some letters in a numeric-only field), verify that error messages are shown properly
          • POST with file upload, e.g. Certificate -> Import
            • valid input (some valid cert)
            • bad input (use some non-cert, e.g. a .gif file or something)

          All these tests should be performed on all supported browser setups, as this code might behave differently on different browsers (although it shouldn't).

          Tested on Fedora 17 with Firefox 16.0.2 and Chromium 20.

          Show
          bbhoffmann Christian Hoffmann added a comment - - edited Fixed in aa71bfdb53ba2151fc0d65389cf7b31f55fa3f3b in the adm-215 branch (not in devel yet!). Our UI response parsing code was wrong, although it worked in most cases. We are using the responseText property of YUI2's async response objects. This apparently works fine for AJAX/XHR requests, but it breaks whenever iframes are used as the request method (YUI2 transparently does that for file uploads). Where "breaks" means: .responseText contains only the text (all HTML stripped) in these cases (as documented...). The correct way to go is to use .responseXML. Sadly, this is not always populated. So the fix is: We try to use .responseXML, if available, but fall back to .responseText in all other cases. This seems to work fine on Firefox and Chrome for all types of requests (GET, POST, POST w/ file upload). As .responseText was used multiple times in ui.js, I moved this decision logic to its own function, which is now used instead. (To avoid code duplication) Notes for testing: The fix touches multiple code paths, which are hit on each and every request. Breakage likelihood is very high, therefore. Tests should be done for all types of requests: GET, i.e. some data table POST, e.g. Zarafa -> Configuration -> Save valid input (verify that changes are saved properly) bad input (e.g. some letters in a numeric-only field), verify that error messages are shown properly POST with file upload, e.g. Certificate -> Import valid input (some valid cert) bad input (use some non-cert, e.g. a .gif file or something) All these tests should be performed on all supported browser setups, as this code might behave differently on different browsers (although it shouldn't). Tested on Fedora 17 with Firefox 16.0.2 and Chromium 20.
          Hide
          bbhoffmann Christian Hoffmann added a comment -

          Both would be fixed by this ticket.

          Show
          bbhoffmann Christian Hoffmann added a comment - Users repeatedly report unexplainable problems regarding certificate uploads (such as https://sourceforge.net/projects/yaffas/forums/forum/1688081/topic/6606609 ) Config restoration being problematic has also been seen "in the wild" Both would be fixed by this ticket.
          Hide
          bbhoffmann Christian Hoffmann added a comment -

          Merged to devel in 4cb7381eb2dc24681cdc56cb75810de3b60ef0ad.

          Show
          bbhoffmann Christian Hoffmann added a comment - Merged to devel in 4cb7381eb2dc24681cdc56cb75810de3b60ef0ad.

            People

            • Assignee:
              Unassigned
              Reporter:
              bbhoffmann Christian Hoffmann
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 5h
                5h
                Remaining:
                Time Spent - 1.75h Remaining Estimate - 3.25h
                3.25h
                Logged:
                Time Spent - 1.75h Remaining Estimate - 3.25h
                1.75h