205890 - Gzipped attachments should get "content-encoding: gzip" and a different MIME type

Status: NEW

(Bugzilla :: Attachments & Requests, defect)

Description

[Boris Zbarsky [:bzbarsky]]

I was reviewing this 600KB patch today.  And I could not use the attachment
editing UI, because the patch had to be gzipped (too big to just attach it).  It
would be nice if such gzipped patches were sent back as text/plain and
"content-encoding: gzip" so that they would properly appear in the iframe and be
editable inline.

Of course, it would be nice to do this for other attachment types too, like
jprof profiles (which are pretty large even if they fit under the normal
attachment limit; if we had this set up, they could always be gzipped before
being uploaded).

Comment 1

[Darin Fisher]

why doesn't bugzilla just compress all patches and text documents instead?

Comment 2

[tenthumbs]

Not everything should be automatically uncompressed, e.g. a tarball, so 
content-encoding should be used sparingly, not on all attachments. 

Don't you really want transfer encoding?

Comment 3

[Boris Zbarsky [:bzbarsky]]

No, we want content-encoding, so that it will NOT be uncompressed when saving. 
That said, no one implements transfer-encoding (no client, no server).  So
content-encoding is used when people do in fact mean transfer encoding.  But
this is not one of those cases.

In any case, this would be user-settable at patch attachment time.  So you could
set application/x-gzip for tarballs if you wanted to (though application/x-tar
with a gzip content-encoding is, in my opinion, more appropriate).

Comment 4

[timeless]

=>Product, needs impl before local install can enable it.

Comment 5

[tenthumbs]

I was objecting to comment 1. Automatically compressing anything is a 
bad idea. You can get decompression bombs that way. I agree it would be 
nice to allow users to specify encoding but that could also lead to 
bombs.

Comment 6

[Darin Fisher]

tenthumbs: i was only referring to text attachments.

Comment 7

[timeless]

darin: note that bugzilla limits non patch attachments to 300k.
To attach a 600k gzip file you need to call it a 'patch' and attach it as
gzip.

Then you'd need to set some magic flag which made bugzilla spit out the right
http headers. (or at the very least uncheck patch and give it a gzip mime type)

As for encoding all files to save 'space'. the problem is that you can actually
ask bugzilla to search through attachment data looking for something, if the
data were to be stored gzipped by default, then that search feature would be
either prohibitively expensive or functionally useless.

I'm commenting in this bug because I actually might post a patch which allows
users to specify a content encoding.

Right now i'm toying with this:
'text/plain, Content-Encoding=gzip' i'll attach a patch which might do it.

Comment 8

[timeless]

Attachment: untested random idea

Comment 9

[timeless]

Comment on attachment 123533 [details] [diff] [review]
untested random idea

opinions?
the idea is that the content type field
becomes an undocumented way to set other fields
offhand i think i need some code to guard
against empty fields.

text/plain, Content-Encoding: gzip,gzip
would be accepted (delim is ', ')

Comment 10

[tenthumbs]

Darin: yes I see. I should wake up before I shoot my mouth off. :-)

The more I think on this the more I think it's a bad idea. Bugzilla's 
are usually open to the public so there's always the possibility of a 
malicious hacker. A compressed text file can expand to something quite 
large. For example, I created a 200 million byte file containing just 
line feed characters. Gzip compresses it to ~190KB. (Because it's so 
repetitive the old compress is actually better, ~36K; and bzip2 is just 
178 bytes). If I could set the encoding and the browser understood it 
then I would fill it up with garbage.

Is anyone really going to check all the attachments? Is bugzilla? Is it 
worth the trouble? 

Comment 11

[Jacob Steenhagen]

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

Comment 12

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 123533 [details] [diff] [review]
untested random idea

The problem of compression bombs is valid but orthogonal (and probably a client
issue, not a Bugzilla issue).  Bugzilla already permits compressed attachments,
and compression bombs can cause just as much damage when downloaded and
decompressed via dedicated decompression programs as when decompressed by a
browser.

I'm not sure I like the idea of overloading the MIME type field with multiple
distinct pieces of data.  I think it'll cause confusion and be difficult to
use, even for the small set of users who will want it.	It also makes searching
for attachments based on their MIME type more cumbersome (although still
doable).

Ideally we should find some way to identify which compressed attachments that
can/should be displayed rather than burdening users with this responsibility,
f.e. by peering into compressed attachments to see if they comprise a single
text file.

Alternately, we might add a checkbox to the UI for identifying such files. 
This has the drawback of adding additional UI for a rarely used feature, which
may be worse than overloading the MIME type field.  If we do overload the MIME
type field, we should make it as easy as possible to use rather than generic,
since there are no other extant applications for a generic header feature and
it seems prone to security vulnerabilities.  One of these might work:

text/plain,gzip
text/plain (gzip)
text/plain+gzip

The latter is intriguing, since it is a legitimate MIME type in its entirety,
but probably causes more problems than it solves.

Note that once we move to the new Bugzilla server (RSN) we should be able to
relax the patch size limitation significantly, although other kinds of
attachments will still have to be restricted so that we don't get a bunch of
2MB screenshots again (yes, we'll also have to fix the patch->non-patch
loophole).

Comment 13

[Gervase Markham [:gerv]]

If we are to overload the MIME type, we should do it in the standard name-value
pair way:

Content-Type: text/html;charset=ISO-8859-1
->
Content-Type: text/plain;transfer-encoding=application/x-gzip

Gerv

Comment 14

[timeless]

Comment on attachment 123533 [details] [diff] [review]
untested random idea

Gerv: i think the only change i'd need for your comment would be this line:
+    ($contenttype, @extras) = split /,\s+/, $contenttype;

(splitting on /;\s*/)

Comment 15

[Gervase Markham [:gerv]]

That wasn't to imply approval of this idea :-) In my view, if you have a 600Kb
patch, you need to do your work in smaller chunks, not fix your bug system :-)

Gerv

Comment 16

[Boris Zbarsky [:bzbarsky]]

Gerv, please suggest to me how you would do the work in bug 125246 in smaller
chunks?  The heart of the patch is the removal of 4000 lines of no-longer-needed
code...

As we move forward with major API changes in layout, such large patches are
going to become somewhat common in the near term...

Comment 17

[Dave Miller [:justdave]]

Comment on attachment 123533 [details] [diff] [review]
untested random idea

myk already gave this a negative review, and his reasoning sounds valid to me,
so I see no reason to override it.

Comment 18

[Brian Murrell]

I'm guessing there has been no movement regarding this general concept?

Beyond patches, we find it frequently desirable to reduce the size of what could be quite a large upload and bugzilla storage requirement by gzipping first (and yielding a 10-or-more-fold savings), but when we view the attachment, we want it unzipped by the browser.

Beyond just the rigmarole of having to unzip and view outside of the browser, sometimes the attachment has a stylesheet reference (in my case an XSL file) from the BZ server, and that fails once the file is local.  So in order to have the file converted to something readable by the XSL file, it has to be (possibly edited first so that it fits within upload limits and) re-uploaded uncompressed.

Is there really no hope of getting official support for this requirement from the Bugzilla team?

Comment 19

[x0]

Note that this also means that attached .svgz files won't be readable by Mozilla as SVG.