You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "mercuryLuz (via GitHub)" <gi...@apache.org> on 2023/07/20 09:24:08 UTC
[GitHub] [commons-imaging] mercuryLuz opened a new pull request, #304: Make checkForSubImage null-safe
mercuryLuz opened a new pull request, #304:
URL: https://github.com/apache/commons-imaging/pull/304
Make checkForSubImage null-safe for TiffImagingParameters params.
Especially as params gets set as null in TiffImageParser.java:183 getAllBufferedImages()
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make checkForSubImage null-safe [commons-imaging]
Posted by "Ditscheridou (via GitHub)" <gi...@apache.org>.
Ditscheridou commented on code in PR #304:
URL: https://github.com/apache/commons-imaging/pull/304#discussion_r1354625456
##########
src/main/java/org/apache/commons/imaging/formats/tiff/TiffImageParser.java:
##########
@@ -75,7 +75,7 @@ private Rectangle checkForSubImage(
// dimensions of the image that is being read. This method
// returns the sub-image specification, if any, and leaves
// further tests to the calling module.
- if (params.isSubImageSet()) {
+ if (params != null && params.isSubImageSet()) {
Review Comment:
This is an actual problem. If you want to read a multipage tiff with:
`TiffImageParser().getAllBufferedImages(File(path))`
you will run into a npe.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make checkForSubImage null-safe [commons-imaging]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #304:
URL: https://github.com/apache/commons-imaging/pull/304#issuecomment-1868315291
> Yeah I can provide that later today. Will open a new PR for that
Hello @Ditscheridou
Please follow up or close this PR. TY!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make checkForSubImage null-safe [commons-imaging]
Posted by "Ditscheridou (via GitHub)" <gi...@apache.org>.
Ditscheridou commented on PR #304:
URL: https://github.com/apache/commons-imaging/pull/304#issuecomment-1757332539
I ran into this today and was just on he way to create a ticket. Any chance this can be merged?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #304: Make checkForSubImage null-safe
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on code in PR #304:
URL: https://github.com/apache/commons-imaging/pull/304#discussion_r1269199173
##########
src/main/java/org/apache/commons/imaging/formats/tiff/TiffImageParser.java:
##########
@@ -75,7 +75,7 @@ private Rectangle checkForSubImage(
// dimensions of the image that is being read. This method
// returns the sub-image specification, if any, and leaves
// further tests to the calling module.
- if (params.isSubImageSet()) {
+ if (params != null && params.isSubImageSet()) {
Review Comment:
Hadn't time to review properly, but it would be useful to check if the method that calls this private method couldn't be updated instead, to avoid a null object reaching here. Thanks
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] mercuryLuz commented on a diff in pull request #304: Make checkForSubImage null-safe
Posted by "mercuryLuz (via GitHub)" <gi...@apache.org>.
mercuryLuz commented on code in PR #304:
URL: https://github.com/apache/commons-imaging/pull/304#discussion_r1269210099
##########
src/main/java/org/apache/commons/imaging/formats/tiff/TiffImageParser.java:
##########
@@ -75,7 +75,7 @@ private Rectangle checkForSubImage(
// dimensions of the image that is being read. This method
// returns the sub-image specification, if any, and leaves
// further tests to the calling module.
- if (params.isSubImageSet()) {
+ if (params != null && params.isSubImageSet()) {
Review Comment:
Thanks for the quick response!
There are multiple calling methods and IMHO it makes the most sense within this method, so that any consumers can just use checkForSubImage(params) to find out whether there's any subImage data to be extracted no matter the given params.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] garydgregory commented on a diff in pull request #304: Make checkForSubImage null-safe
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #304:
URL: https://github.com/apache/commons-imaging/pull/304#discussion_r1270096441
##########
src/main/java/org/apache/commons/imaging/formats/tiff/TiffImageParser.java:
##########
@@ -75,7 +75,7 @@ private Rectangle checkForSubImage(
// dimensions of the image that is being read. This method
// returns the sub-image specification, if any, and leaves
// further tests to the calling module.
- if (params.isSubImageSet()) {
+ if (params != null && params.isSubImageSet()) {
Review Comment:
@mercuryLuz
The only thing missing here is a unit test that shows why this is a problem. Or is this only a problem in theory?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make checkForSubImage null-safe [commons-imaging]
Posted by "Ditscheridou (via GitHub)" <gi...@apache.org>.
Ditscheridou commented on PR #304:
URL: https://github.com/apache/commons-imaging/pull/304#issuecomment-1757347222
Yeah I can provide that later today. Will open a new PR for that
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] garydgregory commented on a diff in pull request #304: Make checkForSubImage null-safe
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #304:
URL: https://github.com/apache/commons-imaging/pull/304#discussion_r1270096441
##########
src/main/java/org/apache/commons/imaging/formats/tiff/TiffImageParser.java:
##########
@@ -75,7 +75,7 @@ private Rectangle checkForSubImage(
// dimensions of the image that is being read. This method
// returns the sub-image specification, if any, and leaves
// further tests to the calling module.
- if (params.isSubImageSet()) {
+ if (params != null && params.isSubImageSet()) {
Review Comment:
The only thing missing here is a unit test that shows why this is a problem.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make checkForSubImage null-safe [commons-imaging]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #304:
URL: https://github.com/apache/commons-imaging/pull/304#issuecomment-1757344878
> I ran into this today and was just on he way to create a ticket. Any chance this can be merged?
Hello @mercuryLuz
Do you have an image that reproduces the issue? Would please create a PR with a unit test?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make checkForSubImage null-safe [commons-imaging]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #304:
URL: https://github.com/apache/commons-imaging/pull/304#issuecomment-1868326825
Closing: In git master now. You are credited in changes.xml.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make checkForSubImage null-safe [commons-imaging]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory closed pull request #304: Make checkForSubImage null-safe
URL: https://github.com/apache/commons-imaging/pull/304
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make checkForSubImage null-safe [commons-imaging]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #304:
URL: https://github.com/apache/commons-imaging/pull/304#discussion_r1425324248
##########
src/main/java/org/apache/commons/imaging/formats/tiff/TiffImageParser.java:
##########
@@ -75,7 +75,7 @@ private Rectangle checkForSubImage(
// dimensions of the image that is being read. This method
// returns the sub-image specification, if any, and leaves
// further tests to the calling module.
- if (params.isSubImageSet()) {
+ if (params != null && params.isSubImageSet()) {
Review Comment:
@Ditscheridou, @mercuryLuz
This PR still needs a test.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org