69306 - [FIX]External apps get spawned with gzip'd files using mod_gzip content-encoding

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect, P1)

Description

[B E W]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; 0.8) Gecko/20010215
BuildID:    2001021508

My web server is set up to serve compressible files as
gzipped files if the user agent claims to accept them.
For example, Micros~1 Word .doc files usually compress 
about 90% and .html files usually compress about 80%.
Pages load faster over any connection, and 3x to 5x faster
over 56k, so this is necessary technology.  The popularity 
of mod_gzip and equivalents is rising rapidly and the
internet experience over 56k is very much improved;  this
is far better than just HTTP1.1 socket keepalive.

Now .html works fine, as do .js and .css.  But when I
download a .doc file from my server, Mozilla launches a
Micros~1 Word window and feeds the file to it.  The only
problem is that Mozilla does not feed Micros~1 Word a
.doc file;  it feeds the raw gzipped file.

So Micros~1 Word comes up with garbage.

This does not happen with Micros~1 Internet Explorer v5.0
because IE5.0 feeds the application the decompressed file.

This does occur in Netscape Navigator 4.7 because NN does
not decompress the file before feeding it to the application.

Note that the gzip'd status of the file is noted in the headers.

This bug is related to bug 51852.

Reproducible: Always
Steps to Reproduce:
1. Go to a mod_gzip enabled web server
2. Download a file that spawns an external viewer, like a
 Micros~1 Word .doc file.

Actual Results:  The file comes up gzip'd

Expected Results:  The file should be decompressed befre being fed to an
external app.

This bug is related to bug 51852.  It is not the same because
this is automatically spawned apps, and that is Save As..., but
the symptom (still compressed files) is the same.

Comment 1

[B E W]

Reproduce at
http://www.emedicolegal.com/IMEReferralform1.doc

Comment 2

[Andreas M. "Clarence" Schneider]

I assume you mean *content* encoding and not *transfer* encoding. Transfer
encoding isn't working at all (bug 59464 and bug 68517). Adjusting summary.

Comment 3

[Keyser Sose]

Confirmed
Platform: PC
OS: Windows 98
Mozilla Build: 2001022007

Marking NEW.

Comment 4

[robbe]

This happens also in the simpler case, where zipping is not applied on-they-fly,
but the content is lying around on the server in this format. Example:

On my webserver, I have some gzipped postscript files, named something.ps.gz.
Apache does the right thing and serves these files with the headers:

Content-Type: application/postscript
Content-Encoding: x-gzip

Mozilla goes and treats the file like application/postscript, a "open or save"
dialog comes up. Saving works fine, as the default filename will end with
".ps.gz" just like in the original. But "open with" will dump the gzipped
content into the tmp area, under randomname.ps. It should be randomname.ps.gz
instead.

(It can be argued that "open with" should save the file in expanded form, but
that's bug 35956's concern)

Comment 5

[Andreas M. "Clarence" Schneider]

Works for me with 2001-05-16-04, Win NT. See my comments in bug 35956.

Comment 6

[benc]

mass move, v2.
qa to me.

Comment 7

[Igor Sysoev]

The most Apaches has following configuration lines:

AddEncoding     x-compress      Z
AddEncoding     x-gzip          gz tgz

This means that response on request for file named as
somename.tar.gz would have header "Content-Encoding: x-gzip".
Mozilla would save this file gzipped. This is right.
MSIE would save it ungzipped. This is wrong.

But if we have Word document (somedoc.doc) gzipped
on the fly by mod_gzip or mod_deflate then MSIE would
save it ungzipped or run MSWord with ungzipped document.
This is right.

Mozilla would save it gzipped or try to run MSWord with gzipped
document. This is wrong.

I think that Mozilla should save or run any response ungzipped only
if it has .gz or .tgz extention or has content type as
"application/x-tar". In other cases Mozilla should behave as MSIE.

Comment 8

[Boris Zbarsky [:bzbarsky]]

I came to the same conclusion as Igor independently a few weeks back.  Talked to
Darin, who said that seems like an acceptable approach...

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch v1.0

Proposed patch.  Darin, Bill, would you review?

Comment 10

[Boris Zbarsky [:bzbarsky]]

As a note, that patch fixes the issues in this bug, bug 120033, bug 121001 and
does not regress bug 116445 or bug 35956.

Comment 11

[Bill Law]

>+static char* nonDecodableTypes [] = {

I think this should be:
  static const char * const nonDecodableTypes [] = {

Couldn't this JS code:
>-      const flags = nsIWBP.PERSIST_FLAGS_NO_CONVERSION | 
nsIWBP.PERSIST_FLAGS_REPLACE_EXISTING_FILES;
>+      webBrowserPersist.persistFlags = 
nsIWBP.PERSIST_FLAGS_REPLACE_EXISTING_FILES;
>+      try {
>+        const helperAppService =
>+          Components.classes["@mozilla.org/uriloader/external-helper-app-
service;1"].getService(Components.interfaces.nsIExternalHelperAppService);
>+        if (helperAppService.applyDecodingForType(persistArgs.contentType)) {
>+          webBrowserPersist.persistFlags &= 
~nsIWBP.PERSIST_FLAGS_NO_CONVERSION;
>+        } else {
>+          webBrowserPersist.persistFlags |= 
nsIWBP.PERSIST_FLAGS_NO_CONVERSION;
>+        }
>+      } catch (e) {
>+        webBrowserPersist.persistFlags |= nsIWBP.PERSIST_FLAGS_NO_CONVERSION;
>+      }
>+      

be simplified:
  webBrowserPersist.persistFlags = nsIWBP.PERSIST_FLAGS_NO_CONVERSION |
                                   nsIWBP.PERSIST_FLAGS_REPLACE_EXISTING_FILES;
  try {
    const helperAppService =
            Components.classes["@mozilla.org/uriloader/external-helper-app-
service;1"].getService(Components.interfaces.nsIExternalHelperAppService);
    if (helperAppService.applyDecodingForType(persistArgs.contentType)) {
      webBrowserPersist.persistFlags &= ~nsIWBP.PERSIST_FLAGS_NO_CONVERSION;
    }
  } catch (e) {
  }

I.e., Always or in PERSIST_FLAGS_NO_CONVERSION and then remove that flag iff 
applyDecodingForType returns false?

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch v1.1

Make the changes Bill suggests.

Comment 13

[Darin Fisher]

mscott should review this...

Comment 14

[Boris Zbarsky [:bzbarsky]]

Attachment: PAtch v1.2

Updated to work with the new world of bug 27609

Comment 15

[Bill Law]

Comment on attachment 70872 [details] [diff] [review]
PAtch v1.2

r=law

Comment 16

[Darin Fisher]

Comment on attachment 70872 [details] [diff] [review]
PAtch v1.2

sr=darin (looks good!)

Comment 17

[Scott MacGregor]

Comment on attachment 70872 [details] [diff] [review]
PAtch v1.2

instead of:
PL_strcasecmp(aMimeContentType, nonDecodableTypes[index]) == 0


how about
if (!PL_strcasecmp(...))

Comment 18

[Boris Zbarsky [:bzbarsky]]

Sure thing, if that's the only issue you have with the patch.  :)

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

a=roc+moz for 0.9.9

Comment 20

[Boris Zbarsky [:bzbarsky]]

checked into trunk for 0.9.9 with mscott's change from "== 0" to "!".

Comment 21

[Boris Zbarsky [:bzbarsky]]

*** Bug 107991 has been marked as a duplicate of this bug. ***