Moodle
  1. Moodle
  2. MDL-32889

Improperly escaped downloads fail in Google Chrome

    Details

    • Testing Instructions:
      Hide

      1. Create an assignment which as at least one comma in the title.
      2. Add one or more submissions to it.
      3. Attempt to download these as a zip file using the current version of Google Chrome.

      Note: this patch touches a couple different areas. The fix specific to mod/assignment is in lib/filelib if this needs to be split up.

      Show
      1. Create an assignment which as at least one comma in the title. 2. Add one or more submissions to it. 3. Attempt to download these as a zip file using the current version of Google Chrome. Note: this patch touches a couple different areas. The fix specific to mod/assignment is in lib/filelib if this needs to be split up.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-32889-master
    • Rank:
      39959

      Description

      Some downloads fail in Google Chome with the error "Error 349 (net::ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_DISPOSITION): Multiple distinct Content-Disposition headers received. This is disallowed to protect against HTTP response splitting attacks." See http://moodle.org/mod/forum/discuss.php?d=201781 for discussion. I can reproduce this with an assignment that has a comma in the same. Enclosing the filename in the Content-Disposition header in quotes resolves the issue.

        Activity

        Hide
        Dan Poltawski added a comment -

        Hmm..

        The RFC doesn't seem to specify quoting
        http://www.ietf.org/rfc/rfc2183.txt

        Show
        Dan Poltawski added a comment - Hmm.. The RFC doesn't seem to specify quoting http://www.ietf.org/rfc/rfc2183.txt
        Hide
        Charles Fulton added a comment -

        Not as far as the filename goes, but there is this bit about parameters generally:
        "NOTE ON PARAMETER VALUE LENGHTS: A short (length <= 78 characters)
        parameter value containing only non-`tspecials' characters SHOULD be
        represented as a single `token'. A short parameter value containing
        only ASCII characters, but including `tspecials' characters, SHOULD
        be represented as `quoted-string'. Parameter values longer than 78
        characters, or which contain non-ASCII characters, MUST be encoded as
        specified in [RFC 2184]."

        In any event, it's more common than not to quote within the existing code.

        Show
        Charles Fulton added a comment - Not as far as the filename goes, but there is this bit about parameters generally: "NOTE ON PARAMETER VALUE LENGHTS: A short (length <= 78 characters) parameter value containing only non-`tspecials' characters SHOULD be represented as a single `token'. A short parameter value containing only ASCII characters, but including `tspecials' characters, SHOULD be represented as `quoted-string'. Parameter values longer than 78 characters, or which contain non-ASCII characters, MUST be encoded as specified in [RFC 2184] ." In any event, it's more common than not to quote within the existing code.
        Hide
        Dan Poltawski added a comment -

        Aha, indeed its quoted all over existing code.

        Thanks. Submitting for integration

        Show
        Dan Poltawski added a comment - Aha, indeed its quoted all over existing code. Thanks. Submitting for integration
        Hide
        Eloy Lafuente (stronk7) added a comment -

        The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

        TIA and ciao

        Show
        Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
        Hide
        Sam Hemelryk added a comment -

        Thanks guys, this has been integrated now

        Show
        Sam Hemelryk added a comment - Thanks guys, this has been integrated now
        Hide
        Andrew Davis added a comment -

        Works as described in Chrome 18.0.1025.168 on Ubuntu 11.04. Passing.

        Show
        Andrew Davis added a comment - Works as described in Chrome 18.0.1025.168 on Ubuntu 11.04. Passing.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        U P S T R E A M I Z E D !

        Many thanks for the hard work, closing this as fixed.

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Many thanks for the hard work, closing this as fixed. Ciao
        Hide
        Jim Peterson added a comment -

        This also is impacting my 2.2 site, is there any chance the same changes can be applied in 2.2?

        Show
        Jim Peterson added a comment - This also is impacting my 2.2 site, is there any chance the same changes can be applied in 2.2?

          People

          • Votes:
            10 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: