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).
why doesn't bugzilla just compress all patches and text documents instead?
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?
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).
=>Product, needs impl before local install can enable it.
Assignee: endico → myk
Component: Bugzilla: Other moz.org Issues → Attachments & Requests
Product: mozilla.org → Bugzilla
QA Contact: myk → matty
Version: other → unspecified
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.
tenthumbs: i was only referring to text attachments.
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 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 ', ')
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?
Attachment #123533 - Flags: review?(justdave) → review?(myk)
*** Bug 207588 has been marked as a duplicate of this bug. ***
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).
Attachment #123533 - Flags: review?(myk) → review-
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 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*/)
Attachment #123533 - Flags: review?(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
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 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.
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?
Note that this also means that attached .svgz files won't be readable by Mozilla as SVG.
You need to log in before you can comment on or make changes to this bug.