Binary data in multipart/x-mixed-replace get an extra line end when saved or viewed.

VERIFIED FIXED in mozilla1.2alpha

Status

()

Core
Networking
P2
major
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: Jesse Houwing, Assigned: Darin Fisher)

Tracking

Trunk
mozilla1.2alpha
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

16 years ago
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1b) Gecko/20020717
BuildID:    2002071708

I've made a piece of coldfusion code that makes it possible to offer multiple
files for download with only one HTTP request.

Text seems to work fine (it doesn't mind an extra line end). But for images and
executables it poses a problem.

I'll attach the exact output shortly.
(Reporter)

Comment 1

16 years ago
Created attachment 92108 [details]
Exact output multipart http file.

This is the output.

The line-end at the end before the final multipart boundary should be there,
but should NOT be part of the binary data.
(Reporter)

Comment 2

16 years ago
My test site isn't always on btw.

Updated

16 years ago
Summary: Binary data in multipart/x-mixed-replace get's an extra lineend added when saved or viewed. → Binary data in multipart/x-mixed-replace get an extra line end when saved or viewed.
(Reporter)

Comment 3

16 years ago
when savinf the file btw, by setting the content type to
application/octet-stream, the resulting file can still not be read by mozilla,
but Internet explorer and Photoshop have no trouble at all.
(Reporter)

Comment 4

16 years ago
hitting back, and then forward does show the image.

I'm sure that is a bug too, but...
(Reporter)

Comment 5

16 years ago
Just tested with 1.1b under XP, and it works there.
(Reporter)

Comment 6

16 years ago
And I tested it at work under 1.1a (2002061404/NT4) and it works there too, but
the latest nightly does not.
(Assignee)

Comment 7

16 years ago
*** Bug 159366 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 8

16 years ago
-> me
Assignee: new-network-bugs → darin

Updated

16 years ago
Blocks: 159366

Comment 9

16 years ago
The dependant bug 159366 is for me a good nsenterprise candidate, it is a
serious concern for us in a deployment for a major french credit insurance
company and I'd like it to be fixed in Netscape 7.

As there is no Priority and no Target Milestone for this bug, it seems to me
nobody is actively working on it.
Is nobody available for that ?
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Keywords: mozilla1.2
Priority: -- → P2
Target Milestone: --- → mozilla1.2alpha
(Assignee)

Comment 10

16 years ago
Created attachment 95114 [details] [diff] [review]
v1 patch

simple patch... we should be stripping LF (or CRLF) preceding each token.  this
is what 4.x appears to do, and it seems to be what's required by RFC 1341
section 7.2.1 (http://www.w3.org/Protocols/rfc1341/7_2_Multipart.html).
(Assignee)

Updated

16 years ago
Keywords: patch

Comment 11

16 years ago
Yes, I forgot in my argumentation that I expected this to be a low impact, low
risk patch.

In the comments for bug 159366, I also quoted RFC 2046 (update to 1341)
explicitly saying that this is the desired behaviour.

Thanks for the fast patch Darin, I hope it can make in releases even before 1.2

Comment 12

16 years ago
Comment on attachment 95114 [details] [diff] [review]
v1 patch

do you really want to have this function inline?  

r=dougt
Attachment #95114 - Flags: review+
(Assignee)

Comment 13

16 years ago
dougt: no, it doesn't have to be inline, but i figured since it's a small
function without any loops, that inlining makes sense.  perhaps i should just
leave it static and let the compiler decide what's best.
Comment on attachment 95114 [details] [diff] [review]
v1 patch

sr=bzbarsky if you change |len--| to |--len|.
Attachment #95114 - Flags: superreview+
(Assignee)

Comment 15

16 years ago
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Keywords: mozilla1.0.1
Resolution: --- → FIXED
(Assignee)

Comment 16

16 years ago
*** Bug 159366 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 17

16 years ago
Jean-Marc: i'm not sure that this will be considered for mozilla1.0.1 since this
problem only applies to multipart/mixed, which isn't even supported by MSIE. 
but, i'll let project management decide.
No longer blocks: 159366
Keywords: adt1.0.1, nsbeta1

Comment 18

16 years ago
My concern is not in fact Mozilla 1.0.1 , but NS 7 final release, because
enterprise users will probably want to use it over Mozilla.

That would be disappointing if NS 7 ships without the patch and I suspect it's
already a bit late, but I'm raising the issue.
Will test the fix when the new nightly is available, should be in a few hours.

Yes, the page where we have the problem is Netscape specific, the IE page uses
VB script, and ActiveX I believe.

Comment 19

16 years ago
Comment on attachment 95114 [details] [diff] [review]
v1 patch

what if cursor > token? Using unsigned arithmetic, you could potentially get a
large 32 bit number in that case and go reading memory not belonging to the
token. If we are going to take a patch like this, it might be good to add a
check for cursor >= token at the start and return 0 if so.
(Assignee)

Comment 20

16 years ago
syd: if what you say could happen then we are no better off without this patch.
 notice that the second parameter to SendData is a PRUint32 and without my patch
we were simply calling SendData(cursor, token - cursor).  clearly, if token <
cursor, then we're screwed anyways.

Comment 21

16 years ago
Let's fix this in Buffy.  -adt
Keywords: adt1.0.1 → adt1.0.1-

Comment 22

16 years ago
Can someone consider this bug for inclusion in 1.0.2 ?
Comment on attachment 95114 [details] [diff] [review]
v1 patch

a=rjesup@wgate.com for 1.0 branch; change mozilla1.0.2+ to fixed1.0.2 when
checked in
Attachment #95114 - Flags: approval+

Updated

16 years ago
Keywords: adt1.0.1-, mozilla1.0.1 → mozilla1.0.2+
(Assignee)

Comment 24

16 years ago
fixed1.0.2
Keywords: mozilla1.0.2+ → fixed1.0.2

Comment 25

16 years ago
benc: can you pls verify this as fixed on the 1.0 branch, then replace
"fixed1.0.2" with "verified1.0.2"? thanks!

Comment 26

16 years ago
What are the STEPS for this bug?

Viewing the link, or saving it?
Mozilla 1.1: viewing results in:
The image, then a text message:
"The image “http://toaom.student.utwente.nl/test.cfm” cannot be displayed,
because it contains errors."

Communicator displays some HTML source before loading the image.


Comment 27

16 years ago
j.houwing@student.utwente.nl or darin -- can you pls answers ben's question, so
that he can verify this issue as fixed? thanks!
(Reporter)

Comment 28

16 years ago
Just tested in:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2b) Gecko/20020924

It should show the picture. if it doesn't and gives an error, then ir's not
working the way it is supposed to.
(Reporter)

Comment 29

16 years ago
It should show some HTML, then some Soure, then the image. Depending on the
speed of your internet connection it could just show an image. If you are
running unpatched it should end with an error saying the image contains an error.

Comment 30

16 years ago
Benjamin, the result you get is the normal result in a fail case.
As 1.1 does not include the patch, this is not surprising. 
Only version from 1.2a, and the 1.0.2 candidates have the patch.

Fail case  :
Some HTML, then some source, shows partially loaded image, then after the image
is fully loaded, sees the unwanted end of line and then complains.

Succes case :
Some HTML, then some source, shows partially loaded image, then stops
downloading and show fully loaded image without problems.

Comment 31

16 years ago
This didn't work in Win32 or Linux branch builds.

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.0.2) Gecko/20020924

That was too close to the checkin, so I updated my Linux build to 20021009, and
it is giving the error. 

I don't know how to reopen for the branch, so I'm reopening the whole bug.

I'm going to move to the trunk now.

Comment 32

16 years ago
Removed the fixed1.0.2 keyword to "re open" the bug on the branch.
Keywords: fixed1.0.2

Comment 33

16 years ago
REOPEN:

Also doesn't work in;

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.2b) Gecko/20021009

I am not seeing much of anything, then the error. No code, no image.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 34

16 years ago
Darin talked to me, it looks like some cache settings from an FTP test affect this.

I'm going to retest, results should be in by tomorrow.

Comment 35

16 years ago
moving back to resolved fixed.
Status: REOPENED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED

Comment 36

16 years ago
VERIFIED/fixed - trunk, mozilla 1.2a all plats.
Status: RESOLVED → VERIFIED

Comment 37

16 years ago
VERIFIED102
allplats 20020924

sorry for the confusion. I'm going to go find that FTP bug that suggested I
should test w/ zero caches, and invalidate it now...
Keywords: verified1.0.2
(Reporter)

Comment 38

16 years ago
Just tested in 2002100808 1.2+ and it still works fro me.
You need to log in before you can comment on or make changes to this bug.