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

Adding Null Pointer check #53

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
@prafulVaishnav

No description provided.

@prafulVaishnav

This comment has been minimized.

Show comment
Hide comment
@prafulVaishnav

prafulVaishnav May 12, 2017

Function ctPageBreak.getVal() (https://github.com/apache/poi/blob/trunk/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFParagraph.java#L824) returns null in some cases. So adding Null Pointer check.

To reproduce the issue, use the attached docx file (sample.docx) and run below code:

InputStream is = new FileInputStream("<attachedFilePath>");
		XWPFDocument docx = new XWPFDocument(is);
		Iterator<XWPFParagraph> paraIter = docx.getParagraphsIterator();
		while (paraIter.hasNext()) {
			XWPFParagraph para = paraIter.next();
			if (para.isEmpty()) {
				//do Something
			}
		}

Function para.isEmpty() will throw NPE.

@tballison @poi-benchmark Please review and merge.

prafulVaishnav commented May 12, 2017

Function ctPageBreak.getVal() (https://github.com/apache/poi/blob/trunk/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFParagraph.java#L824) returns null in some cases. So adding Null Pointer check.

To reproduce the issue, use the attached docx file (sample.docx) and run below code:

InputStream is = new FileInputStream("<attachedFilePath>");
		XWPFDocument docx = new XWPFDocument(is);
		Iterator<XWPFParagraph> paraIter = docx.getParagraphsIterator();
		while (paraIter.hasNext()) {
			XWPFParagraph para = paraIter.next();
			if (para.isEmpty()) {
				//do Something
			}
		}

Function para.isEmpty() will throw NPE.

@tballison @poi-benchmark Please review and merge.

@onealj

This comment has been minimized.

Show comment
Hide comment
@onealj

onealj May 12, 2017

Contributor

Possible side-note, but shouldn't we be checking that the value isn't any of the true-like string values that OOXML STOnOff allows: "true", "1", or "on"?

Contributor

onealj commented May 12, 2017

Possible side-note, but shouldn't we be checking that the value isn't any of the true-like string values that OOXML STOnOff allows: "true", "1", or "on"?

@asfgit asfgit closed this in fac98b5 May 16, 2017

@onealj

This comment has been minimized.

Show comment
Hide comment
@onealj

onealj May 16, 2017

Contributor

Thanks for the pull request and unit test. I didn't include your unit test or sample document in our unit test suite because it's a simple NPE. There weren't any other issues uncovered by the integration testing (org.apache.poi.stress.XWPFFileHandler(document/github53.docx)) with this file.

I also took the opportunity to change isPageBreak to work for all of the boolean-like value: "true", "1", "on", "false", "0", "off".

These changes will be included in POI 3.17 beta 1.

r1795254 https://svn.apache.org/viewvc?view=revision&revision=1795254

Contributor

onealj commented May 16, 2017

Thanks for the pull request and unit test. I didn't include your unit test or sample document in our unit test suite because it's a simple NPE. There weren't any other issues uncovered by the integration testing (org.apache.poi.stress.XWPFFileHandler(document/github53.docx)) with this file.

I also took the opportunity to change isPageBreak to work for all of the boolean-like value: "true", "1", "on", "false", "0", "off".

These changes will be included in POI 3.17 beta 1.

r1795254 https://svn.apache.org/viewvc?view=revision&revision=1795254

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