Closed Bug 105292 (deflate) Opened 23 years ago Closed 22 years ago

Accept-encoding: deflate unsupported, but requested

Categories

(Core :: Networking: HTTP, defect, P3)

Product:
Core
Component:
Networking: HTTP
Platform:
x86
All
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Future

People

(Reporter: m, Assigned: ddick)

References

(
URL
)

Details

(Keywords: topembed+, Whiteboard: [adt2 RTM] [eapp] sun_621 [ETA 08/08])

Attachments

(7 files, 16 obsolete files)

http log for the dflate tranasactions
(deleted), text/plain
Details
deflate compressed file (without http headers)
(deleted), application/octet-stream
Details
Trace file (w/ raw data) of deflate attachment request
(deleted), text/plain
Details
while deflating retry with larger output buffer as long as we get Z_BUF_ERROR
(deleted), patch
Details | Diff | Splinter Review
text.gif from a normal apache server install in icons dir
(deleted), image/gif
Details
cvs diff against tip
(deleted), patch
darin.moz
: review-
Details | Diff | Splinter Review
v2 patch : revised formating
(deleted), patch
dougt
: review+
darin.moz
: superreview+
Details | Diff | Splinter Review
I've made some tests with compressed pages (static and on the fly compressed) and noticed that Mozilla (0.9.4 and 0.9.5) was not able to display the "deflate"- nor the "compress"-version. When I tried to view the source I only got "<html><body></body></html>" the rendered page stood empty. Tried with w3m everything worked fine. Mozilla worked only right with the gzip compressed version. You can see the problem on my testpage: http://future.matthias-wimmer.de/. I have there a sample page in uncompressed (identity), gzip and compress encoding. Only the first two work with Mozilla.
we should either stop sending deflate and compress in the Accept-Encoding, or fix the problem. --> 0.9.7 Btw: i've seen this too.
Severity: normal → major
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P3
Target Milestone: --- → mozilla0.9.7
We have code to support it.
very true... my comments were meant to suggest that unless this can be fixed by the next milestone, advertising support for the feature should simply be disabled.
ack! i lost track of this bug somehow... will make sure it gets some lovin soon.
Priority: P3 → P1
Attached patch changes to make "deflate" compression work (obsolete) (deleted) — — Splinter Review
Here's a patch that enables the "deflate" compression method - basically, all that was missing was the update of uLen when the initial call to uncompress returned Z_BUF_ERROR. I've also disabled HTTP_COMPRESS_COMPRESS, since (as far as I know) there is nothing in zlib that allows decompression of compress(1) content. The only support for this that I could find was in gzip's unlzh.c module, but that only offers a filedesc-to-filedesc interface instead of the memory-to-memory interface that would be needed here.
gereon: thx for figuring out the problem. your patch appears to be backwards... the +'s should be -'s and vice versa. besides that, i thought zlib did support compress, but maybe i'm mistaken. if it doesn't support compress, then you are right to disable support for it, but your patch does not do so correctly. let's just make this patch fix the deflate problem, and file a separate bug to clean up compress... there will be changes that need to be made to the HTTP Accept-Encoding header, for example.
Attached patch updated diffs for nsHTTPCompressConv.cpp/.h (obsolete) (deleted) — — Splinter Review
Sorry for creating any confusion, but I'll have to take my original patch back. it was tested against a buggy HTTP server ... :-( I've now investigated this further and have come up with the attached diffs for nsHTTPCompressConv.cpp/.h (hopefully this time in the right direction). I've also undone my admittedly half-harted attempt at disabling "compress".
Keywords: patch
Whiteboard: [patch needs r/sr=]
-> 0.9.8
Target Milestone: mozilla0.9.7 → mozilla0.9.8
-> vinay
Assignee: darin → badami
Status: ASSIGNED → NEW
This patch does not seem to work. I'am unable to view the "compress" version of the image on http://future.matthias-wimmer.de/ with this. Can u make sure that this patch is working ?
My patch wasn't intended to make "compress" work - it only enables "deflate". In fact, I think supporting compress is not easily possible to do, as I've indicated in a previous comment - I wasn't able to find anything that handles compressed data except for the compress(1) source code. Unfortunately, zlib doesn't do compress. I'd recommend to remove support for compress and leave only gzip and deflate
Is there an URL that I could test deflate against ?
Attached patch to supress compress in accept encoding (obsolete) (deleted) — — Splinter Review
This patch is required to suppres the accept-encoding header from sending compress by default.
Sorry, I'm not aware of any public web servers that support deflate. What I did for my tests was install an Apache with mod_deflate. Unfortunately, that server is not available over the internet, but it's very easy to setup.
this is a blocker for sun/siebel->eapp, nsbeta1
Blocks: 125136
Keywords: nsbeta1
Whiteboard: [patch needs r/sr=] → [patch needs r/sr=][eapp]
Gereon I'am unable to find a mod_deflate. Can u help me with the specifics ?
mod_deflate is available at ftp://ftp.lexa.ru/pub/apache-rus/contrib/.
Gereon Thanks. Got it. However the README seems to be in russian. Can u tell me what u had to do to get it compiled as part of an apache build and what directives u had to use to configure it ? Vinay
I'm pretty busy right now, so apologies for the terseness: # build Apache with mod_deflate: gtar zxvf mod_deflate.1.0.11.tgz cd mod_deflate.1.0.11 configure --with-apache=<apache-dir> make cd <apache-dir> configure --activate-module=src/modules/extra/mod_deflate.o make make install # edit httpd.conf, add to global settings: DeflateEnable on DeflateOrder deflate DeflateMinLength 1 DeflateCompLevel 1 DeflateHTTP 1.0 DeflateTypes text/html text/plain # restart apache # show that it works telnet localhost 8080 GET / HTTP/1.1 Host: localhost Accept-Encoding: deflate prints deflated gibberish on my system Hth!
Attached patch cleaned up patch (obsolete) (deleted) — — Splinter Review
same as earlier with attachment id=59668. Earlier patch had some control characters in it. Seems to work fine. Testing a bit more.
Attached file http log for the dflate tranasactions (deleted) —
In this http log grep for Content-encoding: deflate
Darin can u please review patches 1. to supress compress in accept encoding 2. cleaned up patch
Comment on attachment 70069 [details] [diff] [review] cleaned up patch sr=darin
Attachment #70069 - Flags: superreview+
Gagan, Rick Can one of u r= this one ?
Vinay, I am helping Siebel to test this patch and see if it will fix their deflate problem (for which the esculation was about). After I applied the supress-compress patch (in all.js), what do I need to do? Just recompile libpref, or I have to recompile the whole browser? Thanks in advance.
I think libpref should suffice.
The patch does not seem to fix Siebel's problem. After applying the patchs (supress compression and cleaned up patch) to Netscape6.2.1 on their Solaris 8 machine, when trying to load the login page of Siebel app, inflate() (inside do_deflate() in cleaned up patch) returned -5. Any ideas? This deflate problem currently is Siebel's top bug, preventing them from releasing their app with Netscape6.2.1 on Solaris. Timely help will be greatly appreciated. int nsHTTPCompressConv::do_deflate { ... err = inflate(&stream, Z_FINISH); ... } The stream value was: stream = { z_stream_s::next_in = 0x7afb78 "" z_stream_s::avail_in = 0 z_stream_s::total_in = 1192U z_stream_s::next_out = 0x2c32c7 "" z_stream_s::avail_out = 409U z_stream_s::total_out = 3167U z_stream_s::msg = (nil) z_stream_s::state = 0x7d3e70 z_stream_s::zalloc = 0xfdad7048 = &zcalloc z_stream_s::zfree = 0xfdad705c = &zcfree z_stream_s::opaque = (nil) z_stream_s::data_type = 1192 z_stream_s::adler = 1U z_stream_s::reserved = 4290700296U } z_stream_s::state was: *`libnecko.so`nsHTTPCompressConv.cpp`nsHTTPCompressConv::do_deflate`stream.state = { internal_state::dummy = 7 }
Is there some URL that i can debug this against ?
If u applied the "clened up patch" u would have had to provide necko.dll also. Did u do that or di u only provide libprefs ?
Vinay, we are working to get access to the application for you.
Answers to Vinay's questions: Is there a URL you can access from Netscape? No. Currently there is no external access to their test environment yet, and Siebel is working on it. Is it possible that you can go to Siebel with me? I have the debug environment all set up.Or you can remotely join me debugging. Did you apply neco.dll besides libpref? This problem is on their Solaris machine. After I applied the supress-compression patch, I recompiled libpref, and libneco.so was updated.
New info: If a fix takes some time, Siebel will accept a temparery work around that simply disable defalte, by telling the web sever that the browser can't handle compressed files, so the web server will not send compress files to the browser. Any idea how to do that? If somebody can tell me how to do it, I can apply the changes to the Sun branch.
To disbale defalte temporarily, u need to do the same as what I have done for compress in the "to supress compress in accept encoding" patch. ie +pref("network.http.accept-encoding" ,"gzip;q=0.9"); Sorry, I cannot go to Siebel with u since I work from India. (-:
Keywords: mozilla0.9.9
Error -5 from inflate() is Z_BUF_ERROR which means that the output buffer for decompression of the deflated date was not large enough. Can you try increasing the output buffer size and see if that helps? (e.g. change mOutBufferLen*=3 to *=10 or something) Alternatively, even if there is no outside access to the Siebel test site, could you grab a copy of the raw compressed data (including http headers) using wget or curl and make that available publicly?
Keywords: topembed
Adding a Siebel deflate compressed files (one without http headers, and one with).
The compressed file should be a PDF file (schwab.PDF).
This contains the HTTP trace for deflate attachment request. Includes HTTP header info. and raw compressed data.
Some new finding after further debugging of the Siebel deflate problem: Without the patch, Netscape 6.2.1 on Solaris (or Linux) was able to deflated a compressed login html file and the page was displayed OK (enve though inflate() still returned -5, expending mOutBuffer size from *3 to *10 did not help); but the next compressed PDF (schwab.PDF) file did not got deflated since it did not go through the same deflate method (OnDataAvailable(), where the patch applied) as the login html file did. Should a compressed PDF file (to be deflated)also go through the same deflate method as a compressed html file? If so, where should it be deflated? Any ideas?
Has the patch been tested on Linux? In Siebel's case, there is a Siebel app server that compress files and then send them through the web server to the browser.
Can u just give us the file that is being used for this test in an uncompressed format ?
I just ran some tests on the compressed data. The format is correct, and inflate() decompresses it OK in one go *if* it is given a sufficiently large buffer - slightly more than 82K in this case. All buffer sizes below this limit result in return code -5, everything large give 0, just like it should. My bet is that somehow the buffer size calculation (or size adjustment) failed.
Vinay, We'll post the uncompressed file. Gereon, In our test at Siebel, the PDF file did not go through inflate() in the patch at all. In Siebel's case, their application server sent the compressed file to the web server and then to the browser (as an attachment I believe). Would that make a difference? How was your test setup? Did you apply the patch, and the PDF file just ran through the patch? I'll enlarge the buffer size, and try it again. Last time I enlarged the output buffer from x3 to x10, and inflate() still returned -5. Should I make it bigger?
Comment on attachment 65978 [details] [diff] [review] to supress compress in accept encoding sr=darin
Attachment #65978 - Flags: superreview+
cc'ing bz... bz: FYI the siebel server expects "Content-Encoding: deflate" to be applied to saved content or content fed to a helper app.
Adding AOLTW
Whiteboard: [patch needs r/sr=][eapp] → [patch needs r/sr=][eapp],AOLTW
Darin, do you mean that they expect that the content will be inflated before saving or passing to a helper?
Keywords: nsbeta1+
*** Bug 109279 has been marked as a duplicate of this bug. ***
OS: Linux → All
Could somebody please explain in detail what the problem with Siebel really is? I mean, what is this (non-standard, I think) Attachment: header supposed to mean, and how do they expect the browser to handle is? Display the file? Start Acrobat? Save the file? My tests were run on Linux, and I basically used the code I submitted in the deflate patch to verify that it actually can decompress the data that Siebels's server spits out.
gereon: thanks for testing that... it turns out that siebel expects the data to be inflated before being sent to acroread. currently, mozilla does not inflate when saving to disk or streaming to a helper app. this part of the bug is fixed by bz's patch in bug 128199.
Keywords: nsbeta1
I landed Darin's 094 patch onto the AOLTW branch (minus the NS_NOTREACHED macro).
Plussing for AOLTW
Whiteboard: [patch needs r/sr=][eapp],AOLTW → [patch needs r/sr=][eapp],AOLTW+
The patch for 0.9.4 (provided by Darin Fisher) has been checked into the Sun 6.2.1 branch.
Whiteboard: [patch needs r/sr=][eapp],AOLTW+ → [patch needs r/sr=][eapp],AOLTW+, sun_621
I verified Darin's fix (for 0.9.4) in Siebel today with the new release build.
Rick can u review this one for the trunk please .
Keywords: topembed → topembed+
Whiteboard: [patch needs r/sr=][eapp],AOLTW+, sun_621 → [patch needs r/sr=][eapp],AOLTW+, sun_621 [adt2]
zero'ing the mozilla milestone. we still need this in the mozilla 1.0 branch and trunk though.
Target Milestone: mozilla0.9.8 → ---
Geron Shouldnt the code that does the do_deflate when we get a Z_BUF_ERROR be in a while until we succeed on uncompressing the file or get an error that is not Z_BUF_ERROR? Attaching a patch that does it ? Can u please take a look. Further I'am running into problems with the following gif file. Seems to hog memory and not display it. Can u take a look ? Vinay
Geron This is the same as the cleaned up version of ur patch except that I loop until the do_deflate succeeds with a non-Z_BUF_ERROR. Can u take a look please ? Am I missing something obvious here ?
Geron With this text.gif that comes as part of the normal apache distribution, I seem to be running into problems where the do_delate keeps returning Z_BUF_ERROR even if we loop increasing the output buf size. Unable to figure root cause for this. Any ideas ?
Vinay, the new patch looks OK to me, although I would prefer having some hard upper limit of the decompression buffer. In theory, unlimited reallocation calls might be a risk that can be exploited in a DoS attack. As for the text.gif problem: I've just successfully downloaded this image from my mod_deflate-enabled Apache (with compression for image/gif enabled, although this is rather pointless in practice). Are you sure the tests you ran really used a deflated .gif? Gereon
*** Bug 137244 has been marked as a duplicate of this bug. ***
Attached patch fix for Z_BUF_ERROR loops (obsolete) (deleted) — — Splinter Review
I've investigated the text.gif problem a little further, and it turns out that there is indeed a problem with the way the inflate() buffers were handled. For some files, inflate() needs one dummy byte trailing the compressed data stream, and that is what this patch provides. (Thanks to Mark Adler for pointing me in the right direction).
Attached patch combined patch (obsolete) (deleted) — — Splinter Review
icorporates 79785 + fix to the .h file + 65978 text.gif now works Darin can u please sr this ?
Comment on attachment 80012 [details] [diff] [review] combined patch >Index: netwerk/streamconv/converters/nsHTTPCompressConv.cpp >+ printf("tries = %d inputbuflen=%d outputbuflen=%d\n", >+ tries, mInpBufferLen, mOutBufferLen); this needs to be commented out or #ifdef DEBUG_COMPRESS_CONV or some such #define. >Index: modules/libpref/src/init/all.js >-pref("network.http.accept-encoding" ,"gzip, deflate, compress;q=0.9"); >+pref("network.http.accept-encoding" ,"gzip, deflate;q=0.9"); will we still need this? is compress still broken? otherwise, sr=darin
Attachment #80012 - Flags: superreview+
Darin, since compress is not implemented anywhere, I'd say support for it should be removed entirely.
that does make sense ;-) thx!
-> me
Assignee: badami → darin
Status: NEW → ASSIGNED
Priority: P1 → P2
Whiteboard: [patch needs r/sr=][eapp],AOLTW+, sun_621 [adt2] → [patch needs r/sr=][eapp],AOLTW+, sun_621 [adt2 RTM]
Target Milestone: --- → mozilla1.0.1
Attached patch fix for deflate support (obsolete) (deleted) — — Splinter Review
I had a try at the lastly posted patch (Attachment #80012 [details] [diff]), I'm afraid it doesn't work for me. There seems to be a few oversights in deflate support. First, if the response header contains "Content-Type: dEflate" ("E" is an upper case character), finding the appropriate converter fails because BFSTable for converter chain is an instance of nsObjectHashtable. Second, headers and trailers of deflated data should be expected. RFC2616 defines deflate coding as follows: deflate The "zlib" format defined in RFC 1950 [31] in combination with the "deflate" compression mechanism described in RFC 1951 [29]. In the section 2.2 of RFC1950, the data format is defined to have 2-or 6-byte headers and 4-byte trailers. On the other hand nsHTTPCompressConv::do_deflate calls inflateInit2() with the negative window size to skip over headers and trailers. If it work against some sites, then I doubt it is carried out by a bug of mod_deflate.
wrt the inflateInit() call: when I developed the patch, I initally started with a version that did use the zlib format. It turned out that I wasn't able to find a single web server where this format worked. After much googling I found hints that the RFC is in fact misleading and that no zlib headers are used, so I changed the implementation. Instant success, all deflate servers worked. I also found that the headerless format is supported by the HTTP access functions in Java. If you can provide a URL for a deflating server that does not run Apache/mod_deflate, I'll check again.
Gereon, please try <http://cvs.m17n.org/~akr/diary/> and <http://cvs.m17n.org/~akr/a/>. They are successfully viewed with w3m. Looking over inflate.c coming from w3m-0.3.tar.gz would help you to support the headerless format at the same time. First it tries to decode without headers and trailers, then if inflate() call fails, does try again with a dummy header.
Daiki, thanks for your suggestions. I've incorporated them into the attached patch. Two questions, though: do you know what deflating module is running on cvs.m17.org? It doesn't seem to be mod_deflate, but I'm not aware of any other deflate module. Also, cvs.m17.org seems to ALWAYS generate deflated content, even when the request does not contain an Accept-Encoding header at all. That's just for debugging purposes, I presume?
FYI: MSIE 5.5 displays an empty page for cvs.m17n.org/~akr/diary/. It appears that it only supports the headerless deflate format generated e.g. by mod_deflate.
I thought this was going to go into the 1.0 trunk/branch?
mkaply: it appears to me that there is still ongoing discussion about whether or not this is the right patch. gereon, daiki: please let me know if you are comfortable with the current patch.
Darin, if by current you mean attachment #85171 [details] [diff] [review], I'm comfortable with it, since it allows for both versions of deflated content. Gereon
ok, good... then i will move forward on trying to land that patch. thx for all the help on this one!
Blocks: 143047
Keywords: mozilla0.9.9 → mozilla1.0.1
Whiteboard: [patch needs r/sr=][eapp],AOLTW+, sun_621 [adt2 RTM] → [adt2 RTM] [patch needs r/sr=][eapp],sun_621 [eta needed]
no time now that i'm starting my vacation... -> 1.2alpha
Target Milestone: mozilla1.0.1 → mozilla1.2alpha
*** Bug 160526 has been marked as a duplicate of this bug. ***
*** Bug 160667 has been marked as a duplicate of this bug. ***
*** Bug 160501 has been marked as a duplicate of this bug. ***
*** Bug 160829 has been marked as a duplicate of this bug. ***
*** Bug 160856 has been marked as a duplicate of this bug. ***
*** Bug 160870 has been marked as a duplicate of this bug. ***
*** Bug 160879 has been marked as a duplicate of this bug. ***
*** Bug 160881 has been marked as a duplicate of this bug. ***
darin, what's the current status of this bug and its patch? it's starting to collect duplicates :)
*** Bug 160984 has been marked as a duplicate of this bug. ***
One of the reasons why this bug will collect many dupes is that the Times of India is a very well respected and widely read newspaper in India (paper was launched in 1838) and amongst the Indian diaspora. One can consider this bug as a showstopper for evangalizing Mozilla usage in India and amongst overseas Indians.
*** Bug 161065 has been marked as a duplicate of this bug. ***
*** Bug 161118 has been marked as a duplicate of this bug. ***
*** Bug 161157 has been marked as a duplicate of this bug. ***
*** Bug 161060 has been marked as a duplicate of this bug. ***
*** Bug 161022 has been marked as a duplicate of this bug. ***
this bug is starting to become a _very big_ problem. It it really so difficult to remove 'deflate' this from the accept-header as a workaround for 1.1? see comment #88, why this is so important - currently it's only one page btw...
Keywords: mozilla1.1
Target Milestone: mozilla1.2alpha → mozilla1.1final
*** Bug 161223 has been marked as a duplicate of this bug. ***
Blocks: 1.1
Darin, is this really a 1.1 stopper? It's not a recent regression and it seems to only be highly visible at one website. What's it going to take to fix and how far off is that? We're targeting 1.1 for this week so time is short for any changes here.
It's not a recent regression on our end, but this site just started sending us encoded content... It didn't use to do it.
here's a one-liner that makes timesofindia.com load correctly again. ultimately, i'd like to fixup the stream converter service to properly lowercase fromType and toType before looking for a converter, etc. but, for the sake of moz1.1, i'd recommend this simple patch.
Comment on attachment 94193 [details] [diff] [review] v1 simplest patch required to make timesofindia.com load again sr=bzbarsky
Attachment #94193 - Flags: superreview+
Comment on attachment 94193 [details] [diff] [review] v1 simplest patch required to make timesofindia.com load again r-dougt
Attachment #94193 - Flags: superreview+ → review+
Attachment #94193 - Flags: superreview+
Comment on attachment 94193 [details] [diff] [review] v1 simplest patch required to make timesofindia.com load again a=asa (on behalf of drivers) for checkin to 1.1 branch. Please land this as soon as we have tinderboxen running at http://tinderbox.mozilla.org/showbuilds.cgi?tree=Mozilla1.1
Attachment #94193 - Flags: approval+
Keywords: adt1.0.1
Whiteboard: [adt2 RTM] [patch needs r/sr=][eapp],sun_621 [eta needed] → [adt2 RTM] [eapp] sun_621 [eETA 08/08]
are there any other top sites, besides www.timesofindia.com, that would be benefit from this fix?
Whiteboard: [adt2 RTM] [eapp] sun_621 [eETA 08/08] → [adt2 RTM] [eapp] sun_621 [ETA 08/08]
I've told ADT (via AIM session with Jaime) about certain enterprise customers of Netscape bits (spun of Mozilla branches) that could be affected by this.
checked in attachment 94193 [details] [diff] [review] on the trunk... that doesn't completely fix this bug, but it does take care of timesofindia.com. i'm keeping this bug open in order to address changes required to the stream converter service.
*** Bug 161455 has been marked as a duplicate of this bug. ***
Target Milestone: mozilla1.1final → M1
Target Milestone: M1 → mozilla1.1final
fyi, www.timesofinida.com is working in build 2002080705. Thanks. Removing the URL since it doesn't apply anymore.
URL: http://www.timesofindia.com
Based on Comment #104 and Comment #106, does this still need to be a blocker to v1.1? I'm assuming that the fix is in the v1.1 branch (Darin, correct me if I'm wrong on that).
*** Bug 161489 has been marked as a duplicate of this bug. ***
this hasn't been fixed on the moz1.1 branch yet.
verified that this fixes timesofindia urls on trunk. Tested on winNT4, linux rh6, mac osX using 08/07/2002 builds. Leaving open per comment# 104.
adt1.0.1+ (on ADT's behalf) approval for the checkin of attachment 94193 [details] [diff] [review] (only), to the 1.0 branch, pending Drivers' approval. pls check this in asap, the replace "fixed1.0.1" with "verified1.0.1". thanks!
Keywords: adt1.0.1 → adt1.0.1+
a=chofman for 1.0.1
checked in the fix on the 1.0 branch for mozilla1.0.1
checked in fix on the 1.1 branch.
Keywords: mozilla1.0.1 → fixed1.0.1
verified 1.0 branch - win NT4, mac osX, linux rh6 08/08/02 builds
Keywords: fixed1.0.1 → verified1.0.1
No longer blocks: 1.1
*** Bug 162292 has been marked as a duplicate of this bug. ***
targeting 1.2 alpha for the remaining work...
Target Milestone: mozilla1.1final → mozilla1.2alpha
*** Bug 162673 has been marked as a duplicate of this bug. ***
*** Bug 162880 has been marked as a duplicate of this bug. ***
Gereon: your latest patch (attachment 85171 [details] [diff] [review]) looks like something we might really want to consider adding to mozilla, but it does not apply cleanly at all to the mozilla trunk. any chance you could bring it up-to-date? thx! -> futured for now
Severity: major → normal
Priority: P2 → P3
Target Milestone: mozilla1.2alpha → Future
Darin - I'm off for vacation myself, so I won't be able to work on this in the next two weeks. If anyone else volunteers that's fine with me, otherwise I'll look into when I return.
*** Bug 163100 has been marked as a duplicate of this bug. ***
*** Bug 163346 has been marked as a duplicate of this bug. ***
*** Bug 163484 has been marked as a duplicate of this bug. ***
I am using Mozilla 1.1 (the build released on August 26th) and timesofindia.com does not load in Mozilla. This was working in some pre-release builds of 1.1.
Manoj, Which OS. It works for me on 1.1 release on Linux
sorry, I should have included the OS version in my comment: OS: Win XP Pro It suddenly WFM too... This is interesting. But I checked some 5 times last night before I filed the bug. Dohhh!
Attached patch Updated patch (obsolete) (deleted) — — Splinter Review
Unsure what went wrong with the previos patch, here it is again, applies cleanly to the 1.0 branch (as far as i can tell, nsHTTPCompressConv.cpp/.h hasn't changed since the early 0.9 releases)
*** Bug 173457 has been marked as a duplicate of this bug. ***
Attached patch cvs diff against tip (deleted) — — Splinter Review
Attachment #59510 - Attachment is obsolete: true
Attachment #59668 - Attachment is obsolete: true
Attachment #65978 - Attachment is obsolete: true
Attachment #70069 - Attachment is obsolete: true
Attachment #73092 - Attachment is obsolete: true
Attachment #79785 - Attachment is obsolete: true
Attachment #80012 - Attachment is obsolete: true
Attachment #84890 - Attachment is obsolete: true
Attachment #85171 - Attachment is obsolete: true
Attachment #94193 - Attachment is obsolete: true
Attachment #97301 - Attachment is obsolete: true
Attachment #115388 - Flags: superreview?
Attachment #115388 - Flags: review?(darin)
Comment on attachment 115388 [details] [diff] [review] cvs diff against tip >Index: mozilla/netwerk/streamconv/converters/nsHTTPCompressConv.cpp > if (mOutBuffer == NULL) > return NS_ERROR_OUT_OF_MEMORY; >+ // update uLen as well: >+ uLen = mOutBufferLen; >+ code = do_deflate (mOutBuffer, &uLen, mInpBuffer, streamLen+1); >+ printf("tries = %d inputbuflen=%d outputbuflen=%d\n", >+ tries, mInpBufferLen, mOutBufferLen); indentation is off-by-one. please kill the printf or #ifdef DEBUG. timeless: have you tested this patch?
Attachment #115388 - Flags: review?(darin) → review-
Attachment #115388 - Flags: superreview?
can we please remove the advertising for deflate and compress in time for 1.4? This seems to have been going on forever.
David: deflate and compress work just fine in many cases. i think we're only talking about an edge case here.
g'day Darren, i'm using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3b) Gecko/20030210 and i can't get either compress || deflate to work with it. Lynx works with compress and Opera works with deflate. i got desperate and downloaded the Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3) Gecko/20030304 nightly binary and it didn't work either. Which is why i requested the removal of the Accept-Encoding: deflate,compress options until the issues get ironed out. I take it that the Win32 version is fine?
David: linux version seems to work correctly for me. care to post a testcase?
Following perl cgi script works fine for Opera, nix for moz. #! /usr/local/bin/perl -w use Compress::Zlib(); use strict; MAIN: { my ($html) = <<_HTML_; <html><head><title>Accept-Encoding</title></head> <body> This is the Accept-Encoding test </body> </html> _HTML_ my ($deflatedBody); my ($deflationStream, $status) = Compress::Zlib::deflateInit(); if ($status eq Compress::Zlib::Z_OK()) { my ($out, $status) = $deflationStream->deflate($html); if ($status eq Compress::Zlib::Z_OK()) { $deflatedBody = $out; my ($flush, $status) = $deflationStream->flush(); if ($status eq Compress::Zlib::Z_OK()) { $deflatedBody .= $flush; } else { die('Deflate Failed in flush:' . $status); } } else { die('Deflate Failed:' . $status); } } else { die('Deflate Failed to initialise:' . $status); } print <<_HTTP_; Content-Type: text/html Content-Encoding: deflate _HTTP_ print $deflatedBody; }
David, as I understand it, there are actually several slightly different algorithms that go by the name "compress/deflate". The zlib used in Mozilla handles some of them but not others; it may be that the perl zlib uses some of these others.
AFAIK, Compress::Zlib is just an interface to the zlib library. The calls i used in the test case should therefore translate directly to the zlib library. I don't know enough about compression to quickly compare the deflate definition in rfc2616 with the deflate definition on zlib.org. Can someone supply me with a test case that mozilla does work with? Compress::Zlib doco url follows; http://search.cpan.org/author/PMQS/Compress-Zlib-1.19/Zlib.pm
Following is a perl cgi script to test the compress encoding. It works on Lynx/2.8.4rel.1 libwww-FM/2.14 SSL-MM/1.4.1 OpenSSL/0.9.6b but does not work with moz 1.3b. This time there is less confusion about the method as rfc2616 states that compress is the UNIX utility compress, which is used by this script. #! /usr/local/bin/perl -w use POSIX(); use strict; MAIN: { my ($html) = <<_HTML_; <html><head><title>Accept-Encoding</title></head> <body> This is the Accept-Encoding test </body> </html> _HTML_ my ($fileName) = POSIX::tmpnam(); open(FILE, '>' . $fileName) || die("Failed to open $fileName:$!"); print FILE $html; close(FILE); $ENV{'PATH'} = '/usr/bin'; `/usr/bin/compress $fileName`; open(FILE, $fileName . ".Z") || die("Failed to open $fileName:$!"); my ($compressedBody); while(<FILE>) { $compressedBody .= $_; } close(FILE); unlink($fileName . ".Z"); print <<_HTTP_; Content-Type: text/html Content-Encoding: compress _HTTP_ print $compressedBody; }
This patch has nothing to do whatsoever with "compress", like I mentioned in an earlier comment. I thought that advertising compress in the Accept-Encoding header had been dropped long ago...
if the patch doesn't do something about compress, then it's not fixing this bug as currently defined. either the patch for this bug needs to include a fix to not advertise compress (one of the older patches here seems to have that one line change included), or this bug should be resummarised and a new bug filed for removing the compress thing (which is what Darin suggested way back in comment 6)
From comments 6 and 141, a new bug has been filed for the removal of compress from the Accept-Encoding headers. The bug number is 196406
timeless's (http://bugzilla.mozilla.org/attachment.cgi?id=115388&action=view) patch works for me.
Correction. The patch seems to improve moz's handling of deflate, but it is still failing to handle some pages. Seems to be large pages. Still investigating. Error messages follow. tries = 1 inputbuflen=3764 outputbuflen=33867 tries = 2 inputbuflen=3764 outputbuflen=101601 tries = 3 inputbuflen=3764 outputbuflen=304803 tries = 4 inputbuflen=3764 outputbuflen=914409 tries = 5 inputbuflen=3764 outputbuflen=2743227 tries = 6 inputbuflen=3764 outputbuflen=8229681 tries = 7 inputbuflen=3764 outputbuflen=24689043 tries = 8 inputbuflen=3764 outputbuflen=74067129 tries = 9 inputbuflen=3764 outputbuflen=222201387 tries = 10 inputbuflen=3764 outputbuflen=666604161 Error loading URL http://localhost/deflate : 8007000e (NS_ERROR_OUT_OF_MEMORY)
Summary: Mozilla is sending an Accept-encoding for deflate and compress but doesn't support it. → Accept-encoding: deflate and compress unsupported, but requested
Attached patch patch to make deflate work (obsolete) (deleted) — — Splinter Review
ok, this patch works against mod_deflate when set-up following instructions in comment 19 and against the perl cgi script in comment 136. Previous patches seemed to have a problem when the nsHTTPCompressConv::OnDataAvailable function was called twice for one entity (which can seem to happen for large, for example compressed size > 20k, pages). This patch will also work for these large pages.
modifying summary to reflect that this bug is only about deflate now. (compress has been removed from the accept-encoding in bug 196406)
Keywords: mozilla1.1
Summary: Accept-encoding: deflate and compress unsupported, but requested → Accept-encoding: deflate unsupported, but requested
Attached patch cleaned up patch to make deflate work (obsolete) (deleted) — — Splinter Review
removed a function declaration in the previous patch that was not required from nsHTTPCompressConv.h
Attachment #118891 - Attachment is obsolete: true
Attached patch same patch using "cvs diff -u10" (obsolete) (deleted) — — Splinter Review
created the patch using -u10, tried it out, and it doesn't seem to work. i get a 100% CPU usage spike. seems to be inside nsHTTPCompressConv::OnDataAvailable for ever.
Attachment #118906 - Attachment is obsolete: true
darin, what did you do to make the patch fail? can you tell me more about the example that caused the spike. i have checked everything i can think of, but as stated previously, all the examples discussed in this thread seem to work for me. i've downloaded the latest cvs. cd'ed to netwerk/streamconv/converters applied patch 118984 with patch -p0 <conf.patch cd'ed to base gmake -f client.mk build ./dist/bin/mozilla and pointed it at a fresh install of apache/mod_deflate or at the perl-cgi script or anything else. More information would be appreciated.
Darin. I suspect this patch will stop the spike. Can you let me know if it works?
.
Assignee: darin → david_dick
Status: ASSIGNED → NEW
david: thanks for the update.. i'll give it a try today.
Status: NEW → ASSIGNED
Attached patch Patch with indenting correction (obsolete) (deleted) — — Splinter Review
Darin, can you confirm that the patch works and check the patch in?
Attachment #118984 - Attachment is obsolete: true
Attachment #119430 - Attachment is obsolete: true
Darin, just a reminder about this patch. Could you please review it and check it in to cvs when you get the chance.
Attachment #119430 - Attachment is obsolete: false
Attachment #119430 - Attachment is patch: false
Attachment #119430 - Flags: review?(darin)
Attachment #119797 - Flags: review?(darin)
Attached patch v2 patch : revised formating (deleted) — — Splinter Review
cleaned up the patch a little bit. this testcase works: http://cvs.m17n.org/~akr/diary/ my own testcase created using the unix zip utility (included with RH8 does not). i'm not quite sure why my testcase fails... it hits the endless loop detection code. anyhow, the fact that this patch seems to help mod_deflate is enough of a reason to try to check it in.
Attachment #119430 - Attachment is obsolete: true
Attachment #119797 - Attachment is obsolete: true
Attachment #119430 - Flags: review?(darin)
Attachment #119797 - Flags: review?(darin)
Comment on attachment 120470 [details] [diff] [review] v2 patch : revised formating sr=darin
Attachment #120470 - Flags: superreview+
Attachment #120470 - Flags: review?(dougt)
Comment on attachment 120470 [details] [diff] [review] v2 patch : revised formating cvs -uw patch would probably be better. untabify before checking in. i found at least one tab... +nsHTTPCompressConv::OnDataAvailable (nsIRequest* request, nsISupports *aContext, nsIInputStream *iStr, PRUint32 aSourceOffset, PRUint32 aCount) { nsresult rv = NS_ERROR_FAILURE; - PRUint32 streamLen; + PRUint32 streamLen = aCount; Do you really have to init the buffer: + memset (&d_stream, 0, sizeof (d_stream)); can you kill the space between the function name and the parameter in this file. we are inconsistent.
Attachment #120470 - Flags: review?(dougt) → review+
i think memset is important since inflateInit documentation says: "The fields next_in, avail_in, zalloc, zfree and opaque must be initialized before by the caller."
Checking in nsHTTPCompressConv.cpp; /cvsroot/mozilla/netwerk/streamconv/converters/nsHTTPCompressConv.cpp,v <-- nsHTTPCompressConv.cpp new revision: 1.19; previous revision: 1.18 done Checking in nsHTTPCompressConv.h; /cvsroot/mozilla/netwerk/streamconv/converters/nsHTTPCompressConv.h,v <-- nsHTTPCompressConv.h new revision: 1.8; previous revision: 1.7 done marking FIXED... thanks David!!
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Blocks: 191676
*** Bug 214767 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: