Bug 687416 - Failure to decode JPX with incorrect TNsot in SOT packet
Summary: Failure to decode JPX with incorrect TNsot in SOT packet
Status: NOTIFIED FIXED
Alias: None
Product: JasPer
Classification: Unclassified
Component: Decoder (show other bugs)
Version: 1.701.0
Hardware: PC Linux
: P2 normal
Assignee: Ralph Giles
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-04-08 15:12 UTC by Raph Levien
Modified: 2008-12-19 08:31 UTC (History)
0 users

See Also:
Customer:
Word Size: ---


Attachments
incorrect TNsot work around (791 bytes, patch)
2004-04-09 13:12 UTC, Ralph Giles
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Raph Levien 2004-04-08 15:12:23 UTC
The stream in question is a JPX in the test file
Jhonen%20Vasquez%20-%20Johnny%20The%20Homicidal%20Maniac%20-%2001.pdf
(3930349 bytes total), beginning at byte offset 1846 and continuing
for 207269 bytes. This stream triggers the check on line 503 of
jpc_dec.c, and causes the decode to fail.

Based on my reading of the spec, the file is invalid, because it
specifies an SOT with TPsot = 5 and TNsot = 5 (byte offset 114287
within the stream). Indeed, the tile has six tile parts, but TNsot is
5. It is possible that the author of the file confused the _number_ of
tile parts with the _maximum value_ of the tile part index.

Looking at xpdf 3.00's JPXStream.cc implementation, I see that they
ignore TNsot (nTileParts) altogether. Acrobat Reader 6 also renders
the file without complaint. I think JasPer should similarly tolerate
files with an incorrect TNsot field.

However, JasPer seems to depend on the correctness of TNsot to
sequence its tile part handling logic. In particular, line 621 of
jpc_dec.c uses tile->numparts (TNsot) to decide whether the tile part
is the last one. As a result, the tile is finalized prematurely.

The following patch works around the problem:

*** ref/jasper-1.701.0/src/libjasper/jpc/jpc_dec.c	2004-02-08 17:34:40.000000000
-0800
--- jasper-1.701.0/src/libjasper/jpc/jpc_dec.c	2004-04-08 13:51:38.000000000 -0700
***************
*** 505,509 ****
  	}
  	if (!tile->numparts && sot->numparts > 0) {
! 		tile->numparts = sot->numparts;
  	}
  
--- 505,509 ----
  	}
  	if (!tile->numparts && sot->numparts > 0) {
! 		tile->numparts = sot->numparts + 1;
  	}
  

I don't recommend this patch for inclusion, however, because it is
likely that it will break something else, in particular tile
finalization in valid streams. Rather, I recommend the following:

* Do not use TPsot == TNsot - 1 as the condition for recognizing the
last tile part of a tile. Rather, use Psot == 0 to recognize the case
when the tile part contains all data up to the EOC marker.

* Delay finalization of the tile until it is known that all tile parts
have been seen. I'm not sure exactly when this should be; perhaps at
EOC.
Comment 1 Ralph Giles 2004-04-09 13:12:25 UTC
Created attachment 617 [details]
incorrect TNsot work around

attaching a better patch, as suggested.

We delay finalization to EOC, and modify it to finalize tiles in state
ACTIVELAST as well is just ACTIVE. This has no immediate effect since nothing
else ever looks at that state, but does preserve the label.

We are now relying on the initial setting of tile->numparts = 0 in
jpc_dec_process_siz() when TNsot is zero.

Might be nice to warn on the tile part overrun, but we don't attempt to keep
track.
Comment 2 Ralph Giles 2004-04-28 08:51:09 UTC
I've committed a version of the fix to our branch of jasper. This seems like the
best we can do in terms of quick fixes.