Closed
Bug 158500
Opened 22 years ago
Closed 22 years ago
Binary data in multipart/x-mixed-replace get an extra line end when saved or viewed.
Categories
(Core :: Networking, defect, P2)
Tracking
()
VERIFIED
FIXED
mozilla1.2alpha
People
(Reporter: jesse.houwing, Assigned: darin.moz)
References
()
Details
Attachments
(2 files)
1.95 KB,
application/octet-stream
|
Details | |
1.69 KB,
patch
|
dougt
:
review+
bzbarsky
:
superreview+
jesup
:
approval+
|
Details | Diff | Splinter Review |
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•22 years ago
|
||
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•22 years ago
|
||
My test site isn't always on btw.
Updated•22 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•22 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•22 years ago
|
||
hitting back, and then forward does show the image. I'm sure that is a bug too, but...
Reporter | ||
Comment 5•22 years ago
|
||
Just tested with 1.1b under XP, and it works there.
Reporter | ||
Comment 6•22 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•22 years ago
|
||
*** Bug 159366 has been marked as a duplicate of this bug. ***
Comment 9•22 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•22 years ago
|
Assignee | ||
Comment 10•22 years ago
|
||
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).
Comment 11•22 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•22 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•22 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 14•22 years ago
|
||
Comment on attachment 95114 [details] [diff] [review] v1 patch sr=bzbarsky if you change |len--| to |--len|.
Attachment #95114 -
Flags: superreview+
Assignee | ||
Comment 15•22 years ago
|
||
fixed-on-trunk
Assignee | ||
Comment 16•22 years ago
|
||
*** Bug 159366 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 17•22 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.
Comment 18•22 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•22 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•22 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•22 years ago
|
||
Let's fix this in Buffy. -adt
Comment 22•22 years ago
|
||
Can someone consider this bug for inclusion in 1.0.2 ?
Comment 23•22 years ago
|
||
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•22 years ago
|
Comment 25•22 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•22 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•22 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•22 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•22 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•22 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•22 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•22 years ago
|
||
Removed the fixed1.0.2 keyword to "re open" the bug on the branch.
Keywords: fixed1.0.2
Comment 33•22 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•22 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•22 years ago
|
||
moving back to resolved fixed.
Status: REOPENED → RESOLVED
Closed: 22 years ago → 22 years ago
Resolution: --- → FIXED
Comment 36•22 years ago
|
||
VERIFIED/fixed - trunk, mozilla 1.2a all plats.
Status: RESOLVED → VERIFIED
Comment 37•22 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•22 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.
Description
•