johann.petrak@gmail.com Reporter
	Description • 22 years ago

Seeing this with 2002120422/linux, but this has been reported with other builds
and under other OS too.

The web site http://www.dnbscene.com seems to send a page with a header
  Content-Encoding: gzip
when Mozilla sends a header
   Accept-Encoding: gzip,deflate,compress;q=0.9 
(which is the default).
But the subsequent content is not compressed, but plain text.

Mozilla just shows an empty page and no error message, which is very confusing,
since other browsers show the page just fine. 

Adding the line  
  user_pref("network.http.accept-encoding","compress;q=0.9"); 
in user.js makes the problem go away.

	johann.petrak@gmail.com Reporter
	Comment 1 • 22 years ago

Related bug 144814

	Boris Zbarsky [:bzbarsky]
	Comment 2 • 22 years ago

This is a general issue... what do we do if a stream listener fails to deal with
the data?

	Darin Fisher
	Comment 3 • 22 years ago

the stream converter probably cancels the channel with a non-specific error code
like NS_ERROR_FAILURE (i'm just guessing here), and docshell ignores
non-specific error codes (this is really a bug, but there are too many such
errors passed to docshell that alerting the user would be very annoying... error
pages will solve this problem though).

	johann.petrak@gmail.com Reporter
	Comment 4 • 22 years ago

So it is really two issues - the error code should give information about
the error instead of being nonspecific, and it should be communicated to
the user in some way.
Since Mozilla is often used by developers, I think it would be important to
have informative error messages and as few cases of silent failure as possible.

	Stefan Huszics
	Comment 5 • 22 years ago

> Since Mozilla is often used by developers, I think it would be important to
have informative error messages and as few cases of silent failure as possible.

Indeed.
Also a user sending a developer a mail with "your page has this and this critial
error according to mozilla" instead of a "um, your page doesn't show in my
browser for unknown reason" makes a difference.

In the first case the developer is probably going to fix it, in the second the
user might get a reply like "sorry, probably a browserbug, use IE instead".

	Boris Zbarsky [:bzbarsky]
	Comment 6 • 22 years ago

Luckily, nsIStreamConverter is not frozen.. we could certainly add an error code
to denote "conversion failed" or even something more specific.

That may be a good idea.

Stream converters were sorta next on my hit list anyway... ;)

	David Dick
	Comment 7 • 22 years ago

With a view to providing useful feedback to the user as to why a
content-encoding has failed, can someone point me in the direction of some
instructions, examples, etc of mozilla sending such messages to the user?

I'm ok with the content-encoding stuff now, so if someone could point me in the
right direction for actually communicating the the user?

	Boris Zbarsky [:bzbarsky]
	Comment 8 • 22 years ago

I'd think the best place to add this stuff would be over in
nsWebShell::EndPageLoad.  See the code starting at
http://lxr.mozilla.org/seamonkey/source/docshell/base/nsWebShell.cpp#984

The real work is pretty much all handled by DisplayLoadError -- you may have to
plug a string or two into a properties file.

This function is called from nsDocShell::OnStateChange when it gets STATE_STOP.

So it should be sufficient to make sure that the status for the STATE_STOP is an
error status and then catch it in EndPageLoad...

It looks like the status here is the loadgroup status, which should be coming
from the channel that has the LOAD_DOCUMENT_URI flag (which is exactly what you
want).

So the issue is that the request would need to report an error somehow if a
listener failed... perhaps error returns from OnDataAvailable should terminate
the load and save the error?  Alternately, the converter itself could Cancel()
the request with the proper status.... (this last sounds best to me as far as
not changing any API-stuff in frozen-API land, but it would be cool if channels
noticed OnDataAvailable erroring out....)

	Boris Zbarsky [:bzbarsky]
	Comment 9 • 22 years ago

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

	David Dick
	Comment 10 • 22 years ago

Patch to warn for invalid gzip and deflate encoding methods.  Servers that
produce bzipped, compress(1)ed, etc content encodings will still not be handled
well.  Anyone got any ideas on how to handle content encodings that have not
been requested?

	Boris Zbarsky [:bzbarsky]
	Comment 11 • 22 years ago

Comment on attachment 122387 [details] [diff] [review]
content-encoding error messages

I'm not sure the wording of the error message is optimal; perhaps we can come
up with something that even people who have no understanding of http could
understand....

	Darin Fisher
	Comment 12 • 22 years ago

how about this:

  NS_ERROR_MALFORMED_CONTENT  

and simple error text like this:

  "The page is encoded in a format the browser does not understand."

	David Dick
	Comment 13 • 22 years ago

I think Comment 5 has a valid point.  We should let people know that mozilla is
acting correctly (of course :)) and the site is issuing weird content encoding
commands.  Also, i think (and of course am open to other opinions) that people
might understand the word compression better than anything to do with the more
accurate content encoding.  I would include academic references to back up this
opinion, but...

	Boris Zbarsky [:bzbarsky]
	Comment 14 • 22 years ago

Yeah, we definitely want to make it clear that the problem lies with the site
here....

	Adam Lock
	Comment 15 • 22 years ago

Comment on attachment 122387 [details] [diff] [review]
content-encoding error messages

r=adamlock on the docshell side. The network part looks okay too as far as I
can tell.

	Darin Fisher
	Comment 16 • 22 years ago

Comment on attachment 122387 [details] [diff] [review]
content-encoding error messages

yeah, i agree... my suggested error text isn't what we want here.  was too
early in the morning! ;-)


>Index: docshell/base/nsDocShell.cpp

>+        case NS_ERROR_CONTENT_ENCODING:

but i still think that this error code's name should include the 
word "MALFORMED" or something along those lines.  what's erroneous
about a content encoding?  NS_ERROR_CONTENT_ENCODING doesn't make
sense to me.  how about NS_ERROR_MALFORMED_CONTENT_CODING?  the
HTTP/1.1 standard speaks of "content codings" rather than "content
encodings".  or if we really want to be specific about compression,
then how about:

  NS_ERROR_MALFORMED_COMPRESSION

should this error also be generated if nsHttpChannel cannot create
a stream converter for the given Content-Encoding header?  that
wouldn't exactly be a case of malformed compression, but it would be
included in the cases indicated by the error text below.


>Index: docshell/resources/locale/en-US/appstrings.properties

>+contentEncodingFailed=The site has used a broken or unknown form of compression that the browser could not handle.  The browser has correctly advertised to the site the forms of compression that it can handle, so the site needs to correct it's compression methods and/or policies.

how about this instead:

compressionError=The page you are trying to view cannot be shown because it
uses an invalid or unsupported form of compression.  Please contact the website
to inform them of this problem.


>Index: netwerk/streamconv/converters/nsHTTPCompressConv.h

>+#include "nsNetError.h"

add this to the .cpp file instead.  those who include nsHTTPCompressConv.h do
not need
to include nsNetError.h as well.

	David Dick
	Comment 17 • 22 years ago

Agree that we should also present this error message for content-encodings that
we do not understand, say compress(1) or bzip.	As previously stated in comment
10, i am not sure how/where to go about this.  Suggestions would be welcome.

	Boris Zbarsky [:bzbarsky]
	Comment 18 • 22 years ago

Probably nsHttpChannel::ApplyContentConversions -- that's where the
IsAcceptableEncoding() test would fail if the encoding is not one we accept.

Not sure what we should do if we fail to get the stream converter service there....

	Christian :Biesinger (don't email me, ping me on IRC)
	Comment 19 • 21 years ago

page in url field seems to work now, replacing with new testcase

	Andrew Schultz
	Comment 20 • 20 years ago

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

	Vidar Haarr (not reading bugmail) Assignee
	Comment 21 • 19 years ago

Same as attachment 122612 [details] [diff] [review] updated to trunk and with bz's suggestion in comment
18 implemented.

	Darin Fisher
	Comment 22 • 19 years ago

Comment on attachment 195535 [details] [diff] [review]
with error page if IsAcceptableEncoding() fails.

>Index: netwerk/protocol/http/src/nsHttpChannel.cpp

>+    } else if (val != nsnull) {
>+        return NS_ERROR_MALFORMED_CONTENT_ENCODING;
>     }

In this case the content encoding is actually not malformed.  It is
just unsupported by the browser.

Perhaps we should change the name of the error code to something
less specific.	How about this:

  NS_ERROR_INVALID_CONTENT_ENCODING

Yeah, I like that better.


r=darin with that change.

	Vidar Haarr (not reading bugmail) Assignee
	Comment 23 • 19 years ago

I'll comment in bug 301208 about the new error message in case they want to
give it special review.

	Vidar Haarr (not reading bugmail) Assignee
	Comment 24 • 19 years ago

Maybe
<http://lxr.mozilla.org/seamonkey/source/netwerk/protocol/http/src/nsHttpChannel.cpp#1130>
should use NS_ERROR_INVALID_CONTENT_ENCODING ?

	Boris Zbarsky [:bzbarsky]
	Comment 25 • 19 years ago

Comment on attachment 196134 [details] [diff] [review]
MALFORMED -> INVALID

sr=bzbarsky

	Boris Zbarsky [:bzbarsky]
	Comment 26 • 19 years ago

Comment on attachment 196134 [details] [diff] [review]
MALFORMED -> INVALID

Checked this in on trunk.  Please mark fixed if it is and request 1.8 branch
approval if you feel this is safe and really needed on branch....

	Vidar Haarr (not reading bugmail) Assignee
	Comment 27 • 19 years ago

Thanks, bz!
I don't think this is needed for branch, but Darin would be a better judge of that.

Resolving FIXED for now, please reopen if this is wanted for 1.8.

	Boris Zbarsky [:bzbarsky]
	Comment 28 • 19 years ago

This caused bug 309510