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

      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.

        Gliffy Diagrams

          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: