Bug 51535 - NPOIFSFileSystem seems to parse row numbers as signed
Summary: NPOIFSFileSystem seems to parse row numbers as signed
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: HSSF (show other bugs)
Version: 3.8-dev
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-20 16:26 UTC by Antoni Mylka
Modified: 2011-08-11 11:13 UTC (History)
0 users



Attachments
A test document and a junit test case which exposes the issue (7.25 KB, application/zip)
2011-07-20 16:26 UTC, Antoni Mylka
Details
A patch which seems to fix the issue (1.17 KB, patch)
2011-08-11 10:46 UTC, Antoni Mylka
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoni Mylka 2011-07-20 16:26:31 UTC
Created attachment 27303 [details]
A test document and a junit test case which exposes the issue

When I have a document with data in rows with a high number (say 65535) I get array index exceptions because row numbers are negative. For instance in ValueRecordsAggregate in line

CellValueRecordInterface[] rowCells = records[row];

row is -32768

moreover in HSSFSheet

while (row != null) {
  createRowFromRecord(row);
  row = sheet.getNextRow();
}

There are cases where row.getRowNumber returns a negative int. I tried to manufacture an example file. It seems that NPOIFS somewhere parses the row number as a signed number, while it has to be unsigned. I don't know enough POI to fix this myself but my test case seems to show the problem.

It didn't occur with older POIFS.
Comment 1 Nick Burch 2011-07-21 13:56:50 UTC
I've added a slightly tweaked version of your unit test in r1149181. It's currently disabled as the test fails as described
Comment 2 Antoni Mylka 2011-08-11 10:46:15 UTC
Created attachment 27373 [details]
A patch which seems to fix the issue

NDocumentInputStream.readUShort() method would delegate to LittleIndian.getShort(). I changed this to LittleIndian.getUShort(). Nothing else broke, so the lack of 'U' must have been a typo.
Comment 3 Nick Burch 2011-08-11 11:13:14 UTC
Ah, good spot!

Fixed, and test re-enabled in r1156573, thanks