New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: NumberFormatException if XSSFName.setNameName is set with a long name #55

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@Mille4Ever

Mille4Ever commented May 18, 2017

If you call XSSFName.setNameName with a long value consisting of a letter followed by a big number, you will get a NumberFormatException.

For example:
I want to set the name "F04030020010". In Excel using name box, I can set the name without any problems. If I want set the same name using poi, I will get the exception mentioned above.

The reason for the NumberFormatException:
The method XSSFName.validateName splits the value "F04030020010" in a column part and in a row part. Columns only have letters, rows only numbers. The outcome looks like:
Column = F
Row = 04030020010

In the next step, row will be converted into a number using Integer.parseInt. But the current row value exceeds the max value of an Integer resulting in a NumberFormatException.

Since the logic is fine, I replaced Integer.parseInt with BigDecimal, so there is no problem with parsing big numbers anymore.

fix: NumberFormatException if XSSFName.setNameName is set with a long…
… name which consists of a letter followed by a lot of numbers.
@onealj

This comment has been minimized.

Show comment
Hide comment
@onealj

onealj May 18, 2017

Contributor

Why are we attempting to parse the name as a cell address? Sounds like that is the core of the problem, and changing from Integer to BigInt doesn't address the core problem.

Contributor

onealj commented May 18, 2017

Why are we attempting to parse the name as a cell address? Sounds like that is the core of the problem, and changing from Integer to BigInt doesn't address the core problem.

@Mille4Ever

This comment has been minimized.

Show comment
Hide comment
@Mille4Ever

Mille4Ever May 18, 2017

The reason is described here (https://support.office.com/en-us/article/Define-and-use-names-in-formulas-4d0f13ac-53b7-422e-afd2-abd7ff379c64, Section "Learn about syntax rules for names"):

Cell references disallowed: Names cannot be the same as a cell reference, such as Z$100 or R1C1.

Thus, you have to validate that the given name is not a cell reference.

The reason is described here (https://support.office.com/en-us/article/Define-and-use-names-in-formulas-4d0f13ac-53b7-422e-afd2-abd7ff379c64, Section "Learn about syntax rules for names"):

Cell references disallowed: Names cannot be the same as a cell reference, such as Z$100 or R1C1.

Thus, you have to validate that the given name is not a cell reference.

//Name with very long name. Looks like a cell reference but this is not a reference.
XSSFName nameSheet1 = wb.createName();
nameSheet1.setNameName("F04030020010");

This comment has been minimized.

@pjfanning

pjfanning May 18, 2017

I think it would make sense to adjust setNameName to catch exceptions from isRowWithinRange. isRowWithinRange seems like it likely to be used for me than just this use case.

@pjfanning

pjfanning May 18, 2017

I think it would make sense to adjust setNameName to catch exceptions from isRowWithinRange. isRowWithinRange seems like it likely to be used for me than just this use case.

This comment has been minimized.

@Mille4Ever

Mille4Ever May 19, 2017

You mean to change the code inside XSSFName.setNameName and not altering the test case?

Sorry for my question but your comment is just below the test case, so I'm a little bit confused.

@Mille4Ever

Mille4Ever May 19, 2017

You mean to change the code inside XSSFName.setNameName and not altering the test case?

Sorry for my question but your comment is just below the test case, so I'm a little bit confused.

@onealj

This comment has been minimized.

Show comment
Hide comment
@onealj

onealj May 19, 2017

Contributor

How about private XSSFName.validateName swallows the NumberFormatException?

This will means that public CellReference.cellReferenceIsWithinRange will continue to raise a NFE if it gets a silly input, letting the caller decide how to handle a non-integer row string.

Contributor

onealj commented May 19, 2017

How about private XSSFName.validateName swallows the NumberFormatException?

This will means that public CellReference.cellReferenceIsWithinRange will continue to raise a NFE if it gets a silly input, letting the caller decide how to handle a non-integer row string.

@pjfanning

This comment has been minimized.

Show comment
Hide comment
@pjfanning

pjfanning May 19, 2017

Thanks @onealj - that's basically what I was trying to suggest.
One minor detail is that XSSFName validateName has CellReference.cellReferenceIsWithinRange(col, row, SpreadsheetVersion.EXCEL97) and maybe, this should be EXCEL2007 for XSSF.

Thanks @onealj - that's basically what I was trying to suggest.
One minor detail is that XSSFName validateName has CellReference.cellReferenceIsWithinRange(col, row, SpreadsheetVersion.EXCEL97) and maybe, this should be EXCEL2007 for XSSF.

@asfgit asfgit closed this in 284f7d1 May 19, 2017

@onealj

This comment has been minimized.

Show comment
Hide comment
@onealj

onealj May 19, 2017

Contributor

Danke schön @Mille4Ever für die Pull-Anfrage.

Thanks @pjfanning for pointing out the EXCEL97 error. I fixed that too and grep'd through src/ooxml to make sure there weren't any other oversights in XSSF.

Contributor

onealj commented May 19, 2017

Danke schön @Mille4Ever für die Pull-Anfrage.

Thanks @pjfanning for pointing out the EXCEL97 error. I fixed that too and grep'd through src/ooxml to make sure there weren't any other oversights in XSSF.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment