Closed Bug 9336 Opened 25 years ago Closed 25 years ago

NetContext[A]Sync ignores error codes from imagelib

Categories

(Core :: Layout, defect, P3)

All
Linux
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: newt, Assigned: pnunn)

References

()

Details

(Whiteboard: could be in gfx?)

The image referenced in the URL has an invalid PNG line filter about 75% of the way in, which causes libpng to abort. Even when il_png_write() is modified to return an error value (-1) to its caller, the error code simply gets passed on up the chain until it reaches either ImageConsumer::OnDataAvailable() (reader->Write()) or ImageNetContextSyncImpl::GetURL() (aReader->Write()) and is ignored. As a result, il_png_write() gets called an additional 27 times in this case. I'm not 100% certain there's not another abort mechanism I should be using in lieu of the error return code (maybe IL_ABORTED or IC_ABORT_PENDING?), but it seems fairly obvious from the FirstWrite() code immediately above that Write()'s return code could and should be handled in a similar manner--that is, something like this: ilErr = reader->Write(...); if (ilErr != 0) { result = NS_ERROR_ABORT; break; } This will help speed up error-recovery, obviously.
Depends on: 924
I should note that the test image will crash the browser if the fix to 924 is not in place. I supplied both a mini-patch that takes care of that and a major rewrite of the pngcom code that does so more cleanly, but neither has been checked in as of this evening. In addition, I note that the gfx/src code treats 0 as the "OK" value (which is standard), while the legacy PNG code uses 1. I assume the latter is what needs to be fixed. Finally, my example above is sort of a hybrid of the proper code for each of the two locations; the correct code in each case should be obvious from context (i.e., copy the FirstWrite() code).
Assignee: kmcclusk → pnunn
Target Milestone: M9
Kevin, I'm happy to consider this code my side of the fence. I'm reassigning to me. If you'd like to keep it, let me know. -pnunn
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
fix checked in 08-09-99.
Status: RESOLVED → REOPENED
With the August 11th build (1999081108), the program crashs on Mac 8.6, Win NT, and Win 98 when attempting to load the above url.
Resolution: FIXED → ---
Clearing FIXED resolution due to bug reopen.
hmmmm. I must have some other changes in my local tree. I'm not seeing the crash. -pn
Quick question: does the crash happen WHEN the page is loaded or when you move off of the page? Is any part of the image displayed? ok. so that's 2 questions. But they are both quick. -pn
The page loads first without the image being display. The image then is being drawn (top to bottom) in the window. The crash occurs when the image is 3/4 point of being drawn. I hope this helps you.
ok. I can finally see a crash. on linux...but not on NT. onward -pn
Whiteboard: could be in gfx?
-> m10
I have a fix for this. Waiting for tree to open for m10. (note to self: if.cpp, ipng.cpp, nsPNGDecoder.cpp] -pn
Status: REOPENED → RESOLVED
Closed: 25 years ago25 years ago
Resolution: --- → FIXED
I checked in a fix for this. The imglib displays the image until it hits the error in the corrupted file. It should handle the error by displaying the images' file name. Closing bug as fixed so it gets into the testing cycle. -pn
*** Bug 9000 has been marked as a duplicate of this bug. ***
*** Bug 9478 has been marked as a duplicate of this bug. ***
Pam, the crash no longer occurs in the Aug 24 build. After the image loads, it is replaced with url text. Is this what I should expect ?
yep. The image displays until it hits the corrupted data. Then it displays the broken/error image info, which appears to be the filename these days. I'm not sure who decided on the change....may be the layout guys. -pn
Status: RESOLVED → VERIFIED
Fixed in the August 24th Build.
Blocks: 15163
You need to log in before you can comment on or make changes to this bug.