29643 - IMG src can *only* handle image content types.

Status: VERIFIED FIXED

(Core :: Networking, defect, P3)

Description

[Judson Valeski]

This URL sticks a cgi URL in an IMG src tag. The cgi returns data as the
"multipart/x-mixed-replaced" content type. The current URI loader implementation
considers the image lib as the load initiator of the cgi and hands the data off
to it. The image lib doesn't know how to handle x-mixed-replaced and
subsequently chokes on it.

Comment 1

[Judson Valeski]

I just tried having the ImageConsumer::CanHandleContent() reject
"multipart/x-mixed-replaced" in hopes that would kick the load out to another
context (and that context would be able to load the type), to no avail. I
suspect we'll need to have the ImageConsumer reject types that it indeed can't
handle, *and* we'll need to rework the URI loader a bit.

Comment 2

[kiel]

If you add this instead



  if (nsCRT::strcasecmp(aContentType, "multipart/x-mixed-replace") == 0)

    *aDesiredContentType = nsCRT::strdup("*/*");



you will call on the stream converter.  I have added a patch in bug 15625 that 

allows the stream converter to handle binary data (but it needs to be made 

current i believe).



There are a number of bugs that report this, the start of the problem is this 

one.  The next problem was that nsMultiMixedConv choked on binary data because it 

used string types.  If you fix the first two you can get one image out of the 

stream.  The final issue is that the imagelib/jpeglib or some component used in 

ImageConsumer can't handle multiple OnStart/OnStop calls from the stream 

converter (which was mentioned in bug #18330).  Something is broken along the way 

that breaks re-entry.  I've tried to track it down but i've had other things to 

attend to.

Comment 3

[Judson Valeski]

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

Comment 4

[Judson Valeski]

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

Comment 5

[Judson Valeski]

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

Comment 6

[Eli Goldberg]

Tom, when you verify this bug, could you please also double-check duplicate bug
#15625?

(If you're pressed for time when you do the verification, please feel free to QA
assign it over to me when you're done, and I'll double-check 15625.)

Thanks!

Comment 7

[Warren Harris]

Moving to M16.

Comment 8

[Judson Valeski]

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

Comment 9

[Judson Valeski]

ok, so binary data works now in the multi mixed converter (will checkin when 
tree opens).

So here's the current problem. Because the imagelib takes on the url load itself 
(i.e. the uri loader isn't involved) without any intermediary stream conversion 
(ie to convert the multi-mixed), it chokes when it tries to parse an image out 
of the multipart/x-mixed-replace data.

Could/should img src= loads go through the URI loader (this would "do the right 
thing" because the URI loader envokes the converter? Maybe HTTP should 
implicitly do the multipart/x... conversion for everyone?????? Scott?

Comment 10

[pnunn]

Jud:

I'm working on some big changes for image lib.
I was thinking of changing the url to uri for
IL_GetImage(), so there is less code dancing when
I get an animated, redirected image url.

Would this change help you as well in this circumstance?
-P

Comment 11

[Judson Valeski]

no. the problem here is data consumption related WRT data targeting.

Comment 12

[kiel]

At /gfx/src/nsImageNetContextAsync.cpp#511 the ImageConsumer calls the image 

reader to stop loading with a fixed argument that this is not a multipart image: 

/modules/libimg/src/ilNetReader.cpp#126



If you arrange for it to tell it correctly whether or not it is a multipart, the 

second image that loads will crash the browser.  What happens is the jpeg object 

or something poking at the structure deletes some pointers or such the first time 

StreamComplete is called.



The URI loader is involved in loading the image, that is what calls 

ImageConsumer::CanHandleContent() and inserts the stream converter if necessary.



I'd have traces to share or better a fix but my low tech equipment makes it slow 

and tedious.



PS don't believe everything I say

Comment 13

[kiel]

not sure why it double spaces everything I write, sorry

Comment 14

[pnunn]

Kiel:
I'll take a look at the changes you mention in nsImageNetContextAsync.cpp
tomorrow. Any other test urls you can pass this way??
-P

Comment 15

[James Rome]

Yes, we have yet another type of streaming media that also does not display:
http://www.ornl.gov/doe2k/mmc_cameras/

Comment 16

[kiel]

minimal test case: http://sporkism.org/~kiel/test.html

Comment 17

[pnunn]

I finally got the changes checked in. 
My changes to add FlushImgBuffer() are in.
And the changes you sent to me in gfx/src/nsImageNetContextAsync.cpp 
are in.

Pam

Comment 18

[leger]

We need current retest on this please.

Comment 19

[gerardok]

Cannot retest because of a blocker that Pam Nunn is working on. -> 37909

Comment 20

[James Rome]

http://bant.ms.ornl.gov/galwebv/lists (Webcast)
still does not work with build 15 (33112)
http://tpm.amc.anl.gov/TPMLVideo.html (GTS) is also dead.

Comment 21

[WD]

Bug 37909 was recently fixed....     Any progress on this bug?

-WD

Comment 22

[Judson Valeski]

done

Comment 23

[Tom Everingham]

verif.
Linux 2000060620

Comment 24

[James Rome]

I just retested on Build 2000071910.
http://www-lmes1.ornl.gov/doe2k/highway1/  does not work still

It is vital for our collaboratory that this be fixed!

Comment 25

[Judson Valeski]

broken

Comment 26

[Judson Valeski]

This bug is still fixed. I opened a new one which reflects the actual problem.
see 47215

Comment 27

[Tom Everingham]

verif.