You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/04/13 18:39:41 UTC
[GitHub] [commons-io] ottobackwards opened a new pull request #112: IO-665
ensure that passing a null InputStream results in NPE with tests
ottobackwards opened a new pull request #112: IO-665 ensure that passing a null InputStream results in NPE with tests
URL: https://github.com/apache/commons-io/pull/112
This pr adds a null check for the InputStream constructor of XmlStreamReader. This makes it so that each contractor of type ( File, URL, URLConnection, InputStream ) throws NPE if passed a NULL for that type.
Tests were added as well for each type.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] garydgregory merged pull request #112: IO-665 ensure
that passing a null InputStream results in NPE with tests
Posted by GitBox <gi...@apache.org>.
garydgregory merged pull request #112: IO-665 ensure that passing a null InputStream results in NPE with tests
URL: https://github.com/apache/commons-io/pull/112
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] coveralls edited a comment on issue #112: IO-665
ensure that passing a null InputStream results in NPE with tests
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #112: IO-665 ensure that passing a null InputStream results in NPE with tests
URL: https://github.com/apache/commons-io/pull/112#issuecomment-613043647
[](https://coveralls.io/builds/30054936)
Coverage increased (+0.1%) to 89.739% when pulling **2dcd21b60c8a6929d5fb312e9f585b4f622a2c8b on ottobackwards:IO-665** into **6a78ef8903ef7c2c785b8d0cc08a150d1e2ba818 on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] garydgregory commented on a change in pull request
#112: IO-665 ensure that passing a null InputStream results in NPE with tests
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #112: IO-665 ensure that passing a null InputStream results in NPE with tests
URL: https://github.com/apache/commons-io/pull/112#discussion_r407647154
##########
File path: src/test/java/org/apache/commons/io/input/XmlStreamReaderTest.java
##########
@@ -78,6 +82,26 @@ protected void _testRawNoBomInvalid(final String encoding) throws Exception {
}
}
+ @Test
+ protected void testNullFileInput() throws IOException {
+ assertThrows(NullPointerException.class, ()-> new XmlStreamReader((File)null));
Review comment:
Hi @ottobackwards
The spacing in "() ->" is inconsistent, it should be "() ->" everywhere.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] garydgregory commented on issue #112: IO-665 ensure
that passing a null InputStream results in NPE with tests
Posted by GitBox <gi...@apache.org>.
garydgregory commented on issue #112: IO-665 ensure that passing a null InputStream results in NPE with tests
URL: https://github.com/apache/commons-io/pull/112#issuecomment-613177116
> That is because you @garydgregory know java better than I lol. Will do
;-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] elharo commented on a change in pull request #112:
IO-665 ensure that passing a null InputStream results in NPE with tests
Posted by GitBox <gi...@apache.org>.
elharo commented on a change in pull request #112: IO-665 ensure that passing a null InputStream results in NPE with tests
URL: https://github.com/apache/commons-io/pull/112#discussion_r407709475
##########
File path: src/test/java/org/apache/commons/io/input/XmlStreamReaderTest.java
##########
@@ -78,6 +82,26 @@ protected void _testRawNoBomInvalid(final String encoding) throws Exception {
}
}
+ @Test
+ protected void testNullFileInput() throws IOException {
+ assertThrows(NullPointerException.class, () -> new XmlStreamReader((File)null));
Review comment:
Has commons-io decided to move Java 8 minimum? Maven still requires Java 7 and could not upgrade to this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] ottobackwards commented on a change in pull request
#112: IO-665 ensure that passing a null InputStream results in NPE with tests
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #112: IO-665 ensure that passing a null InputStream results in NPE with tests
URL: https://github.com/apache/commons-io/pull/112#discussion_r407745191
##########
File path: src/test/java/org/apache/commons/io/input/XmlStreamReaderTest.java
##########
@@ -78,6 +82,26 @@ protected void _testRawNoBomInvalid(final String encoding) throws Exception {
}
}
+ @Test
+ protected void testNullFileInput() throws IOException {
+ assertThrows(NullPointerException.class, () -> new XmlStreamReader((File)null));
Review comment:
```xml
<properties>
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>
<commons.componentid>io</commons.componentid>
<commons.module.name>org.apache.commons.io</commons.module.name>
<commons.rc.version>RC1</commons.rc.version>
<commons.bc.version>2.6</commons.bc.version>
<commons.release.version>2.7</commons.release.version>
<commons.release.desc>(requires Java 8)</commons.release.desc>
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] coveralls edited a comment on issue #112: IO-665
ensure that passing a null InputStream results in NPE with tests
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #112: IO-665 ensure that passing a null InputStream results in NPE with tests
URL: https://github.com/apache/commons-io/pull/112#issuecomment-613043647
[](https://coveralls.io/builds/30048747)
Coverage increased (+0.2%) to 89.784% when pulling **73666acb69870b42ff55f6d1f8d7ed5c4b165703 on ottobackwards:IO-665** into **6a78ef8903ef7c2c785b8d0cc08a150d1e2ba818 on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] ottobackwards commented on issue #112: IO-665 ensure
that passing a null InputStream results in NPE with tests
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on issue #112: IO-665 ensure that passing a null InputStream results in NPE with tests
URL: https://github.com/apache/commons-io/pull/112#issuecomment-613041315
Actually, since they use
` this(object.somethingThatReturnsAStream())` we would have to change the initialization pattern from this() to some sort of initialize pattern.
Do you have a preference in that area?
They _behave_ correctly, but there is no message.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] coveralls commented on issue #112: IO-665 ensure that
passing a null InputStream results in NPE with tests
Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #112: IO-665 ensure that passing a null InputStream results in NPE with tests
URL: https://github.com/apache/commons-io/pull/112#issuecomment-613043647
[](https://coveralls.io/builds/30048477)
Coverage increased (+0.1%) to 89.72% when pulling **73666acb69870b42ff55f6d1f8d7ed5c4b165703 on ottobackwards:IO-665** into **6a78ef8903ef7c2c785b8d0cc08a150d1e2ba818 on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] garydgregory commented on issue #112: IO-665 ensure
that passing a null InputStream results in NPE with tests
Posted by GitBox <gi...@apache.org>.
garydgregory commented on issue #112: IO-665 ensure that passing a null InputStream results in NPE with tests
URL: https://github.com/apache/commons-io/pull/112#issuecomment-613140349
> Actually, since they use
>
> ` this(object.somethingThatReturnsAStream())` we would have to change the initialization pattern from this() to some sort of initialize pattern.
>
> Do you have a preference in that area?
>
> They _behave_ correctly, but there is no message.
I do not see why you'd need anything more than changing methods like:
```
public XmlStreamReader(final URL url) throws IOException {
this(url.openConnection(), null);
}
```
to:
```
public XmlStreamReader(final URL url) throws IOException {
this(Objects.requireNonNull(url, "url").openConnection(), null);
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] ottobackwards commented on issue #112: IO-665 ensure
that passing a null InputStream results in NPE with tests
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on issue #112: IO-665 ensure that passing a null InputStream results in NPE with tests
URL: https://github.com/apache/commons-io/pull/112#issuecomment-613167635
@garydgregory done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] ottobackwards commented on issue #112: IO-665 ensure
that passing a null InputStream results in NPE with tests
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on issue #112: IO-665 ensure that passing a null InputStream results in NPE with tests
URL: https://github.com/apache/commons-io/pull/112#issuecomment-613039169
The other contructors, already throw the NPE so I didn't change them. We could add the check to ensure that thread is always a message though.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-io] ottobackwards commented on issue #112: IO-665 ensure
that passing a null InputStream results in NPE with tests
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on issue #112: IO-665 ensure that passing a null InputStream results in NPE with tests
URL: https://github.com/apache/commons-io/pull/112#issuecomment-613165102
That is because you @garydgregory know java better than I lol. Will do
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services