Bug 105292 (deflate)

Accept-encoding: deflate unsupported, but requested

RESOLVED FIXED in Future

Status

()

P3
normal
RESOLVED FIXED
18 years ago
5 years ago

People

(Reporter: m, Assigned: ddick)

Tracking

({topembed+})

Trunk
Future
x86
All
topembed+
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt2 RTM] [eapp] sun_621 [ETA 08/08], URL)

Attachments

(7 attachments, 16 obsolete attachments)

316.78 KB, text/plain
Details
62.12 KB, application/octet-stream
Details
63.30 KB, text/plain
Details
4.52 KB, patch
Details | Diff | Splinter Review
229 bytes, image/gif
Details
6.93 KB, patch
darin.moz
: review-
Details | Diff | Splinter Review
23.84 KB, patch
dougt
: review+
darin.moz
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

18 years ago
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.

Comment 1

18 years ago
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.

Comment 3

18 years ago
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.

Comment 4

18 years ago
ack!  i lost track of this bug somehow... will make sure it gets some lovin soon.
Priority: P3 → P1

Comment 5

18 years ago
Created attachment 59510 [details] [diff] [review]
changes to make "deflate" compression work

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.

Comment 6

18 years ago
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.

Comment 7

18 years ago
Created attachment 59668 [details] [diff] [review]
updated diffs for nsHTTPCompressConv.cpp/.h

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".

Updated

17 years ago
Keywords: patch
Whiteboard: [patch needs r/sr=]

Comment 8

17 years ago
-> 0.9.8
Target Milestone: mozilla0.9.7 → mozilla0.9.8

Comment 9

17 years ago
-> vinay
Assignee: darin → badami
Status: ASSIGNED → NEW

Comment 10

17 years ago
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 ?

Comment 11

17 years ago
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

Comment 12

17 years ago
Is there an  URL that I could test deflate against ?

Comment 13

17 years ago
Created attachment 65978 [details] [diff] [review]
to supress compress in accept encoding

This patch is required to suppres the accept-encoding header from sending
compress by default.

Comment 14

17 years ago
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.

Comment 15

17 years ago
this is a blocker for sun/siebel->eapp, nsbeta1
Blocks: 125136
Keywords: nsbeta1
Whiteboard: [patch needs r/sr=] → [patch needs r/sr=][eapp]

Comment 16

17 years ago
Gereon
I'am unable to find a mod_deflate. Can u help me with the specifics ?

Comment 17

17 years ago
mod_deflate is available at ftp://ftp.lexa.ru/pub/apache-rus/contrib/.

Comment 18

17 years ago
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

Comment 19

17 years ago
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!

Comment 20

17 years ago
Created attachment 70069 [details] [diff] [review]
cleaned up patch

same as earlier with attachment id=59668.
Earlier patch had some control characters in it.

Seems to work fine.
Testing a bit more.

Comment 21

17 years ago
Created attachment 70071 [details]
http log for the dflate tranasactions

In this http log grep for 
Content-encoding: deflate

Comment 22

17 years ago
Darin 
can u please review patches
1. to supress compress in accept encoding
2. cleaned up patch

Comment 23

17 years ago
Comment on attachment 70069 [details] [diff] [review]
cleaned up patch

sr=darin
Attachment #70069 - Flags: superreview+

Comment 24

17 years ago
Gagan, Rick
Can one of u r= this one ?

Comment 25

17 years ago
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.

Comment 26

17 years ago
I think libpref should suffice.

Comment 27

17 years ago
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
}

Comment 28

17 years ago
Is there some URL that i can debug this against ?

Comment 29

17 years ago
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 ?

Comment 30

17 years ago
Vinay, we are working to get access to the application for you.

Comment 31

17 years ago
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.

Comment 32

17 years ago
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. 

Comment 33

17 years ago
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. (-:

Updated

17 years ago
Keywords: mozilla0.9.9

Comment 34

17 years ago
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?

Updated

17 years ago
Keywords: topembed

Comment 35

17 years ago
Created attachment 72178 [details]
deflate compressed file (without http headers)

Adding a Siebel deflate compressed files (one without http headers, and one
with).

Comment 36

17 years ago
The compressed file should be a PDF file (schwab.PDF).

Comment 37

17 years ago
Created attachment 72183 [details]
Trace file (w/ raw data) of deflate attachment request

This contains the HTTP trace for deflate attachment request.  Includes HTTP
header info. and raw compressed data.

Comment 38

17 years ago
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?

Comment 39

17 years ago
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. 

Comment 40

17 years ago
Can u just give us the file that is being used for this test in an uncompressed
format ? 

Comment 41

17 years ago
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.

Comment 42

17 years ago
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 43

17 years ago
Comment on attachment 65978 [details] [diff] [review]
to supress compress in accept encoding

sr=darin
Attachment #65978 - Flags: superreview+

Comment 44

17 years ago
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.

Comment 45

17 years ago
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?

Updated

17 years ago
Keywords: nsbeta1+
*** Bug 109279 has been marked as a duplicate of this bug. ***

Updated

17 years ago
OS: Linux → All

Comment 48

17 years ago
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.

Comment 49

17 years ago
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.

Updated

17 years ago
Keywords: nsbeta1

Comment 50

17 years ago
Created attachment 73092 [details] [diff] [review]
patch for 0.9.4 branch (back-port of the critical part of bz's patch for bug 128199)

Comment 51

17 years ago
I landed Darin's 094 patch onto the AOLTW branch (minus the NS_NOTREACHED macro).

Comment 52

17 years ago
Plussing for AOLTW
Whiteboard: [patch needs r/sr=][eapp],AOLTW → [patch needs r/sr=][eapp],AOLTW+

Comment 53

17 years ago
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

Comment 54

17 years ago
I verified Darin's fix (for 0.9.4) in Siebel today with the new release build.

Comment 55

17 years ago
Rick can u review this one for the trunk please .

Updated

17 years ago
Keywords: topembed → topembed+

Updated

17 years ago
Whiteboard: [patch needs r/sr=][eapp],AOLTW+, sun_621 → [patch needs r/sr=][eapp],AOLTW+, sun_621 [adt2]

Comment 56

17 years ago
zero'ing the mozilla milestone. we still need this in the mozilla 1.0 branch and
trunk though.
Target Milestone: mozilla0.9.8 → ---

Comment 57

17 years ago
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

Comment 58

17 years ago
Created attachment 78706 [details] [diff] [review]
while deflating retry with larger output buffer as long as we get Z_BUF_ERROR

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 ?

Comment 59

17 years ago
Created attachment 78707 [details]
text.gif from a normal apache server install in icons dir

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 ?

Comment 60

17 years ago
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. ***

Comment 62

17 years ago
Created attachment 79785 [details] [diff] [review]
fix for Z_BUF_ERROR loops

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).

Comment 63

17 years ago
Created attachment 80012 [details] [diff] [review]
combined patch

icorporates 79785 + fix to the .h file + 65978
text.gif now works
Darin can u please sr this ?

Comment 64

17 years ago
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+

Comment 65

17 years ago
Darin, since compress is not implemented anywhere, I'd say support for it 
should be removed entirely.

Comment 66

17 years ago
that does make sense ;-)  thx!

Comment 67

17 years ago
-> me
Assignee: badami → darin

Updated

17 years ago
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

Comment 68

17 years ago
Created attachment 84890 [details] [diff] [review]
fix for deflate support

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.

Comment 69

17 years ago
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.

Comment 70

17 years ago
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.

Comment 71

17 years ago
Created attachment 85171 [details] [diff] [review]
proposed patch to accept deflate with and without zlib headers

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?

Comment 72

17 years ago
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?

Comment 74

17 years ago
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.

Comment 75

17 years ago
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

Comment 76

17 years ago
ok, good... then i will move forward on trying to land that patch.  thx for all
the help on this one!

Updated

17 years ago
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]

Comment 77

17 years ago
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. ***

Comment 88

17 years ago
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.

Comment 89

17 years ago
*** Bug 161065 has been marked as a duplicate of this bug. ***

Comment 90

17 years ago
*** 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...

Updated

17 years ago
Keywords: mozilla1.1
Target Milestone: mozilla1.2alpha → mozilla1.1final

Comment 95

17 years ago
*** Bug 161223 has been marked as a duplicate of this bug. ***

Updated

17 years ago
Blocks: 157592
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.

Comment 98

17 years ago
Created attachment 94193 [details] [diff] [review]
v1 simplest patch required to make timesofindia.com load again

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+

Updated

17 years ago
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]

Comment 103

17 years ago
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.

Comment 104

17 years ago
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.

Comment 105

17 years ago
*** Bug 161455 has been marked as a duplicate of this bug. ***

Updated

17 years ago
Target Milestone: mozilla1.1final → M1

Updated

17 years ago
Target Milestone: M1 → mozilla1.1final

Comment 106

17 years ago
fyi, www.timesofinida.com is working in build 2002080705. Thanks. Removing the
URL since it doesn't apply anymore.

Comment 107

17 years ago
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).

Comment 108

17 years ago
*** Bug 161489 has been marked as a duplicate of this bug. ***

Comment 109

17 years ago
this hasn't been fixed on the moz1.1 branch yet.

Comment 110

17 years ago
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+

Comment 112

17 years ago
a=chofman for 1.0.1

Comment 113

17 years ago
checked in the fix on the 1.0 branch for mozilla1.0.1

Comment 114

17 years ago
checked in fix on the 1.1 branch.

Updated

17 years ago
Keywords: mozilla1.0.1 → fixed1.0.1

Comment 115

17 years ago
verified 1.0 branch - win NT4, mac osX, linux rh6 08/08/02 builds
Keywords: fixed1.0.1 → verified1.0.1

Updated

17 years ago
No longer blocks: 157592

Comment 116

17 years ago
*** Bug 162292 has been marked as a duplicate of this bug. ***

Comment 117

17 years ago
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. ***

Comment 119

17 years ago
*** Bug 162880 has been marked as a duplicate of this bug. ***

Comment 120

17 years ago
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

Comment 121

17 years ago
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. ***

Comment 125

17 years ago
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

Comment 127

17 years ago
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! 

Comment 128

17 years ago
Created attachment 97301 [details] [diff] [review]
Updated patch

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)

Comment 129

17 years ago
*** Bug 173457 has been marked as a duplicate of this bug. ***

Comment 130

16 years ago
Created attachment 115388 [details] [diff] [review]
cvs diff against tip
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

Updated

16 years ago
Attachment #115388 - Flags: superreview?
Attachment #115388 - Flags: review?(darin)

Comment 131

16 years ago
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-

Updated

16 years ago
Attachment #115388 - Flags: superreview?
(Assignee)

Comment 132

16 years ago
can we please remove the advertising for deflate and compress in time for 1.4?
This seems to have been going on forever.

Comment 133

16 years ago
David: deflate and compress work just fine in many cases.  i think we're only
talking about an edge case here.
(Assignee)

Comment 134

16 years ago
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?

Comment 135

16 years ago
David: linux version seems to work correctly for me.  care to post a testcase?
(Assignee)

Comment 136

16 years ago
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.
(Assignee)

Comment 138

16 years ago
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
(Assignee)

Comment 139

16 years ago
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;
}

Comment 140

16 years ago
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...

Comment 141

16 years 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)
(Assignee)

Comment 142

16 years ago
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
(Assignee)

Comment 143

16 years ago
timeless's 
(http://bugzilla.mozilla.org/attachment.cgi?id=115388&action=view)
 patch works for me.
(Assignee)

Comment 144

16 years ago
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)

Updated

16 years ago
Summary: Mozilla is sending an Accept-encoding for deflate and compress but doesn't support it. → Accept-encoding: deflate and compress unsupported, but requested
(Assignee)

Comment 146

16 years ago
Created attachment 118891 [details] [diff] [review]
patch to make deflate work

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.

Comment 147

16 years ago
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
(Assignee)

Comment 148

16 years ago
Created attachment 118906 [details] [diff] [review]
cleaned up patch to make deflate work

removed a function declaration in the previous patch that was not required from
nsHTTPCompressConv.h
(Assignee)

Updated

16 years ago
Attachment #118891 - Attachment is obsolete: true

Comment 149

16 years ago
Created attachment 118984 [details] [diff] [review]
same patch using "cvs diff -u10"

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
(Assignee)

Comment 150

16 years ago
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.
(Assignee)

Comment 151

16 years ago
Created attachment 119430 [details]
Patch with check for non-deflated data advertised as deflated

Darin.	I suspect this patch will stop the spike.  Can you let me know if it
works?

Comment 152

16 years ago
.
Assignee: darin → david_dick
Status: ASSIGNED → NEW

Comment 153

16 years ago
david: thanks for the update.. i'll give it a try today.
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 154

16 years ago
Created attachment 119797 [details] [diff] [review]
Patch with indenting correction

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
(Assignee)

Comment 155

16 years ago
Darin, just a reminder about this patch.  Could you please review it and check
it in to cvs when you get the chance.  

Updated

16 years ago
Attachment #119430 - Attachment is obsolete: false
Attachment #119430 - Attachment is patch: false
Attachment #119430 - Flags: review?(darin)
Attachment #119797 - Flags: review?(darin)

Comment 156

16 years ago
Created attachment 120470 [details] [diff] [review]
v2 patch : revised formating

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

Updated

16 years ago
Attachment #119430 - Flags: review?(darin)

Updated

16 years ago
Attachment #119797 - Flags: review?(darin)

Comment 157

16 years ago
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+

Comment 159

16 years ago
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."

Comment 160

16 years ago
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
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Assignee)

Updated

16 years ago
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.