[jira] [Closed] (POOL-411) NPE when deregistering key at end of borrow
[
https://issues.apache.org/jira/browse/POOL-411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Phil Steitz closed POOL-411.
> NPE when deregistering key at end of borrow
> ---
>
> Key: POOL-411
> URL: https://issues.apache.org/jira/browse/POOL-411
> Project: Commons Pool
> Issue Type: Task
>Affects Versions: 2.11.1
>Reporter: Richard Eckart de Castilho
>Priority: Major
> Fix For: 2.12.0
>
>
> There is a potential for an NPE happening in the finally block of
> borrowObject:
> {noformat}
> Caused by: java.lang.NullPointerException: Cannot invoke
> "org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getNumInterested()"
> because "objectDeque" is null
> at
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.deregister(GenericKeyedObjectPool.java:821)
> at
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.borrowObject(GenericKeyedObjectPool.java:507)
> at
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.borrowObject(GenericKeyedObjectPool.java:350)
>
> {noformat}
> From reading the code, it seems this could happen e.g. if a pool is
> concurrently cleared while a borrow is in progress.
> Not sure what a proper solution here would be. Maybe deregister should
> silently do nothing if poolMap.get(k) returns null?
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342196281
##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVp8x.java:
##
@@ -42,76 +45,105 @@
* }
*
* @see https://developers.google.com/speed/webp/docs/riff_container#extended_file_format;>Extended
File Format
- *
* @since 1.0-alpha4
*/
-public final class WebPChunkVP8X extends WebPChunk {
-private final boolean hasICC;
+public final class WebPChunkVp8x extends WebPChunk {
+private final boolean hasIcc;
private final boolean hasAlpha;
private final boolean hasExif;
private final boolean hasXmp;
-private final boolean isAnimation;
+private final boolean hasAnimation;
private final int canvasWidth;
private final int canvasHeight;
-public WebPChunkVP8X(int type, int size, byte[] bytes) throws
ImagingException {
+/**
+ * Create a VP8x chunk.
+ *
+ * @param type VP8X chunk type
+ * @param size VP8X chunk size
+ * @param bytes VP8X chunk data
+ * @throws ImagingException if the chunk data and the size provided do not
match,
+ * or if the other parameters provided are
invalid.
+ */
+public WebPChunkVp8x(int type, int size, byte[] bytes) throws
ImagingException {
super(type, size, bytes);
if (size != 10) {
throw new ImagingException("VP8X chunk size must be 10");
}
int mark = bytes[0] & 0xFF;
-this.hasICC = (mark & 0b0010_) != 0;
+this.hasIcc = (mark & 0b0010_) != 0;
this.hasAlpha = (mark & 0b0001_) != 0;
this.hasExif = (mark & 0b_1000) != 0;
this.hasXmp = (mark & 0b_0100) != 0;
-this.isAnimation = (mark & 0b_0010) != 0;
+this.hasAnimation = (mark & 0b_0010) != 0;
-this.canvasWidth = (bytes[4] & 0xFF) + ((bytes[5] & 0xFF) << 8) +
((bytes[6] & 0xFF) << 16) + 1;
-this.canvasHeight = (bytes[7] & 0xFF) + ((bytes[8] & 0xFF) << 8) +
((bytes[9] & 0xFF) << 16) + 1;
+this.canvasWidth = SafeOperations.add((bytes[4] & 0xFF), ((bytes[5] &
0xFF) << 8), ((bytes[6] & 0xFF) << 16), 1);
+this.canvasHeight = SafeOperations.add((bytes[7] & 0xFF), ((bytes[8] &
0xFF) << 8), ((bytes[9] & 0xFF) << 16), 1);
Review Comment:
Thanks @Glavo ! Will update it over next day(s) :+1:
--
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-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342195072
##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVp8x.java:
##
@@ -42,76 +45,105 @@
* }
*
* @see https://developers.google.com/speed/webp/docs/riff_container#extended_file_format;>Extended
File Format
- *
* @since 1.0-alpha4
*/
-public final class WebPChunkVP8X extends WebPChunk {
-private final boolean hasICC;
+public final class WebPChunkVp8x extends WebPChunk {
+private final boolean hasIcc;
private final boolean hasAlpha;
private final boolean hasExif;
private final boolean hasXmp;
-private final boolean isAnimation;
+private final boolean hasAnimation;
private final int canvasWidth;
private final int canvasHeight;
-public WebPChunkVP8X(int type, int size, byte[] bytes) throws
ImagingException {
+/**
+ * Create a VP8x chunk.
+ *
+ * @param type VP8X chunk type
+ * @param size VP8X chunk size
+ * @param bytes VP8X chunk data
+ * @throws ImagingException if the chunk data and the size provided do not
match,
+ * or if the other parameters provided are
invalid.
+ */
+public WebPChunkVp8x(int type, int size, byte[] bytes) throws
ImagingException {
super(type, size, bytes);
if (size != 10) {
throw new ImagingException("VP8X chunk size must be 10");
}
int mark = bytes[0] & 0xFF;
-this.hasICC = (mark & 0b0010_) != 0;
+this.hasIcc = (mark & 0b0010_) != 0;
this.hasAlpha = (mark & 0b0001_) != 0;
this.hasExif = (mark & 0b_1000) != 0;
this.hasXmp = (mark & 0b_0100) != 0;
-this.isAnimation = (mark & 0b_0010) != 0;
+this.hasAnimation = (mark & 0b_0010) != 0;
-this.canvasWidth = (bytes[4] & 0xFF) + ((bytes[5] & 0xFF) << 8) +
((bytes[6] & 0xFF) << 16) + 1;
-this.canvasHeight = (bytes[7] & 0xFF) + ((bytes[8] & 0xFF) << 8) +
((bytes[9] & 0xFF) << 16) + 1;
+this.canvasWidth = SafeOperations.add((bytes[4] & 0xFF), ((bytes[5] &
0xFF) << 8), ((bytes[6] & 0xFF) << 16), 1);
+this.canvasHeight = SafeOperations.add((bytes[7] & 0xFF), ((bytes[8] &
0xFF) << 8), ((bytes[9] & 0xFF) << 16), 1);
Review Comment:
The other changes look good. I’m going to take a break too, and I’ll revisit
the entire PR later to see what I haven’t forgotten.
##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVp8x.java:
##
@@ -42,76 +45,105 @@
* }
*
* @see https://developers.google.com/speed/webp/docs/riff_container#extended_file_format;>Extended
File Format
- *
* @since 1.0-alpha4
*/
-public final class WebPChunkVP8X extends WebPChunk {
-private final boolean hasICC;
+public final class WebPChunkVp8x extends WebPChunk {
+private final boolean hasIcc;
private final boolean hasAlpha;
private final boolean hasExif;
private final boolean hasXmp;
-private final boolean isAnimation;
+private final boolean hasAnimation;
private final int canvasWidth;
private final int canvasHeight;
-public WebPChunkVP8X(int type, int size, byte[] bytes) throws
ImagingException {
+/**
+ * Create a VP8x chunk.
+ *
+ * @param type VP8X chunk type
+ * @param size VP8X chunk size
+ * @param bytes VP8X chunk data
+ * @throws ImagingException if the chunk data and the size provided do not
match,
+ * or if the other parameters provided are
invalid.
+ */
+public WebPChunkVp8x(int type, int size, byte[] bytes) throws
ImagingException {
super(type, size, bytes);
if (size != 10) {
throw new ImagingException("VP8X chunk size must be 10");
}
int mark = bytes[0] & 0xFF;
-this.hasICC = (mark & 0b0010_) != 0;
+this.hasIcc = (mark & 0b0010_) != 0;
this.hasAlpha = (mark & 0b0001_) != 0;
this.hasExif = (mark & 0b_1000) != 0;
this.hasXmp = (mark & 0b_0100) != 0;
-this.isAnimation = (mark & 0b_0010) != 0;
+this.hasAnimation = (mark & 0b_0010) != 0;
-this.canvasWidth = (bytes[4] & 0xFF) + ((bytes[5] & 0xFF) << 8) +
((bytes[6] & 0xFF) << 16) + 1;
-this.canvasHeight = (bytes[7] & 0xFF) + ((bytes[8] & 0xFF) << 8) +
((bytes[9] & 0xFF) << 16) + 1;
+this.canvasWidth = SafeOperations.add((bytes[4] & 0xFF), ((bytes[5] &
0xFF) << 8), ((bytes[6] & 0xFF) << 16), 1);
+this.canvasHeight = SafeOperations.add((bytes[7] & 0xFF), ((bytes[8] &
0xFF) << 8), ((bytes[9] & 0xFF) << 16), 1);
Review Comment:
The other changes look good. I’m going to take a break too, and I’ll revisit
the entire PR later to see what I haven’t forgotten.
--
This is an automated message from the Apache Git Service.
To respond to
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342194529
##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVp8x.java:
##
@@ -42,76 +45,105 @@
* }
*
* @see https://developers.google.com/speed/webp/docs/riff_container#extended_file_format;>Extended
File Format
- *
* @since 1.0-alpha4
*/
-public final class WebPChunkVP8X extends WebPChunk {
-private final boolean hasICC;
+public final class WebPChunkVp8x extends WebPChunk {
+private final boolean hasIcc;
private final boolean hasAlpha;
private final boolean hasExif;
private final boolean hasXmp;
-private final boolean isAnimation;
+private final boolean hasAnimation;
private final int canvasWidth;
private final int canvasHeight;
-public WebPChunkVP8X(int type, int size, byte[] bytes) throws
ImagingException {
+/**
+ * Create a VP8x chunk.
+ *
+ * @param type VP8X chunk type
+ * @param size VP8X chunk size
+ * @param bytes VP8X chunk data
+ * @throws ImagingException if the chunk data and the size provided do not
match,
+ * or if the other parameters provided are
invalid.
+ */
+public WebPChunkVp8x(int type, int size, byte[] bytes) throws
ImagingException {
super(type, size, bytes);
if (size != 10) {
throw new ImagingException("VP8X chunk size must be 10");
}
int mark = bytes[0] & 0xFF;
-this.hasICC = (mark & 0b0010_) != 0;
+this.hasIcc = (mark & 0b0010_) != 0;
this.hasAlpha = (mark & 0b0001_) != 0;
this.hasExif = (mark & 0b_1000) != 0;
this.hasXmp = (mark & 0b_0100) != 0;
-this.isAnimation = (mark & 0b_0010) != 0;
+this.hasAnimation = (mark & 0b_0010) != 0;
-this.canvasWidth = (bytes[4] & 0xFF) + ((bytes[5] & 0xFF) << 8) +
((bytes[6] & 0xFF) << 16) + 1;
-this.canvasHeight = (bytes[7] & 0xFF) + ((bytes[8] & 0xFF) << 8) +
((bytes[9] & 0xFF) << 16) + 1;
+this.canvasWidth = SafeOperations.add((bytes[4] & 0xFF), ((bytes[5] &
0xFF) << 8), ((bytes[6] & 0xFF) << 16), 1);
+this.canvasHeight = SafeOperations.add((bytes[7] & 0xFF), ((bytes[8] &
0xFF) << 8), ((bytes[9] & 0xFF) << 16), 1);
Review Comment:
There is no need to use `SafeOperations` here. Both `canvasWidth ` and
`canvasHeight ` are 24-bit integers, so overflow is not possible.
I forget why I want to check that they are non-negative, but I guess using
`SafeOperations` would mislead the reader that an overflow might occur here, so
it might be better to revert it.
--
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-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342193272
##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunk.java:
##
@@ -29,30 +30,50 @@
* used by the parser.
*
* @see https://developers.google.com/speed/webp/docs/riff_container;>WebP
Container Specification
- *
* @since 1.0-alpha4
*/
public abstract class WebPChunk extends BinaryFileParser {
private final int type;
private final int size;
protected final byte[] bytes;
+private int chunkSize;
Review Comment:
Good catch! 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.
To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342193216
##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -0,0 +1,351 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.webp;
+
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.ImageInfo;
+import org.apache.commons.imaging.ImageParser;
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.common.XmpEmbeddable;
+import org.apache.commons.imaging.common.XmpImagingParameters;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffImageParser;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunk;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANIM;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANMF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkEXIF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkICCP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8L;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8X;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXMP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXYZW;
+
+import java.awt.Dimension;
+import java.awt.image.BufferedImage;
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import static org.apache.commons.imaging.common.BinaryFunctions.read4Bytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.readBytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.skipBytes;
+
+public class WebPImageParser extends ImageParser
implements XmpEmbeddable {
+
+private static final String DEFAULT_EXTENSION =
ImageFormats.WEBP.getDefaultExtension();
+private static final String[] ACCEPTED_EXTENSIONS =
ImageFormats.WEBP.getExtensions();
+
+@Override
+public WebPImagingParameters getDefaultParameters() {
+return new WebPImagingParameters();
+}
+
+@Override
+public String getName() {
+return "WebP-Custom";
+}
+
+@Override
+public String getDefaultExtension() {
+return DEFAULT_EXTENSION;
+}
+
+@Override
+protected String[] getAcceptedExtensions() {
+return ACCEPTED_EXTENSIONS;
+}
+
+@Override
+protected ImageFormat[] getAcceptedTypes() {
+return new ImageFormat[]{ImageFormats.WEBP};
+}
+
+static int readFileHeader(InputStream is) throws IOException,
ImageReadException {
+byte[] buffer = new byte[4];
+if (is.read(buffer) < 4 ||
!WebPConstants.RIFF_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");
+}
+
+int fileSize = read4Bytes("File Size", is, "Not a Valid WebP File",
ByteOrder.LITTLE_ENDIAN);
+if (fileSize < 0) {
+throw new ImageReadException("File Size is too long:" + fileSize);
+}
+
+if (is.read(buffer) < 4 ||
!WebPConstants.WEBP_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");
Review Comment:
All good over here. The Imaging classes called by users throw
`ImagingException`. The `BinaryFunctions` and other classes may raise
`IOException`, but that would be either wrapped in an ImagingException or
raised directly to the user. At least that was the original design of the
component code, if I recall correctly, so we just follow it (more or less).
Marking as resolved :+1:
--
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-unsubscr...@commons.apache.org
For queries about this service, please
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342193049
##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -0,0 +1,351 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.webp;
+
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.ImageInfo;
+import org.apache.commons.imaging.ImageParser;
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.common.XmpEmbeddable;
+import org.apache.commons.imaging.common.XmpImagingParameters;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffImageParser;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunk;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANIM;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANMF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkEXIF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkICCP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8L;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8X;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXMP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXYZW;
+
+import java.awt.Dimension;
+import java.awt.image.BufferedImage;
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import static org.apache.commons.imaging.common.BinaryFunctions.read4Bytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.readBytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.skipBytes;
+
+public class WebPImageParser extends ImageParser
implements XmpEmbeddable {
+
+private static final String DEFAULT_EXTENSION =
ImageFormats.WEBP.getDefaultExtension();
+private static final String[] ACCEPTED_EXTENSIONS =
ImageFormats.WEBP.getExtensions();
+
+@Override
+public WebPImagingParameters getDefaultParameters() {
+return new WebPImagingParameters();
+}
+
+@Override
+public String getName() {
+return "WebP-Custom";
+}
+
+@Override
+public String getDefaultExtension() {
+return DEFAULT_EXTENSION;
+}
+
+@Override
+protected String[] getAcceptedExtensions() {
+return ACCEPTED_EXTENSIONS;
+}
+
+@Override
+protected ImageFormat[] getAcceptedTypes() {
+return new ImageFormat[]{ImageFormats.WEBP};
+}
+
+static int readFileHeader(InputStream is) throws IOException,
ImageReadException {
+byte[] buffer = new byte[4];
+if (is.read(buffer) < 4 ||
!WebPConstants.RIFF_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");
+}
+
+int fileSize = read4Bytes("File Size", is, "Not a Valid WebP File",
ByteOrder.LITTLE_ENDIAN);
+if (fileSize < 0) {
+throw new ImageReadException("File Size is too long:" + fileSize);
+}
+
+if (is.read(buffer) < 4 ||
!WebPConstants.WEBP_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");
+}
+
+return fileSize;
+}
+
+@Override
+public WebPImageMetadata getMetadata(ByteSource byteSource,
WebPImagingParameters params) throws ImageReadException, IOException {
+try (ChunksReader reader = new ChunksReader(byteSource,
WebPChunkXMP.TYPE_EXIF)) {
+WebPChunk chunk = reader.readChunk();
+return chunk == null ? null : new
WebPImageMetadata((TiffImageMetadata) new
TiffImageParser().getMetadata(chunk.getBytes()));
+}
+}
+
+@Override
+public String getXmpXml(ByteSource byteSource, XmpImagingParameters
params) throws ImageReadException, IOException {
+try (ChunksReader reader =
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342192794
##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -0,0 +1,351 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.webp;
+
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.ImageInfo;
+import org.apache.commons.imaging.ImageParser;
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.common.XmpEmbeddable;
+import org.apache.commons.imaging.common.XmpImagingParameters;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffImageParser;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunk;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANIM;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANMF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkEXIF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkICCP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8L;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8X;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXMP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXYZW;
+
+import java.awt.Dimension;
+import java.awt.image.BufferedImage;
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import static org.apache.commons.imaging.common.BinaryFunctions.read4Bytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.readBytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.skipBytes;
+
+public class WebPImageParser extends ImageParser
implements XmpEmbeddable {
+
+private static final String DEFAULT_EXTENSION =
ImageFormats.WEBP.getDefaultExtension();
+private static final String[] ACCEPTED_EXTENSIONS =
ImageFormats.WEBP.getExtensions();
+
+@Override
+public WebPImagingParameters getDefaultParameters() {
+return new WebPImagingParameters();
+}
+
+@Override
+public String getName() {
+return "WebP-Custom";
+}
+
+@Override
+public String getDefaultExtension() {
+return DEFAULT_EXTENSION;
+}
+
+@Override
+protected String[] getAcceptedExtensions() {
+return ACCEPTED_EXTENSIONS;
+}
+
+@Override
+protected ImageFormat[] getAcceptedTypes() {
+return new ImageFormat[]{ImageFormats.WEBP};
+}
+
+static int readFileHeader(InputStream is) throws IOException,
ImageReadException {
+byte[] buffer = new byte[4];
+if (is.read(buffer) < 4 ||
!WebPConstants.RIFF_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");
+}
+
+int fileSize = read4Bytes("File Size", is, "Not a Valid WebP File",
ByteOrder.LITTLE_ENDIAN);
+if (fileSize < 0) {
+throw new ImageReadException("File Size is too long:" + fileSize);
+}
+
+if (is.read(buffer) < 4 ||
!WebPConstants.WEBP_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");
+}
+
+return fileSize;
+}
+
+@Override
+public WebPImageMetadata getMetadata(ByteSource byteSource,
WebPImagingParameters params) throws ImageReadException, IOException {
+try (ChunksReader reader = new ChunksReader(byteSource,
WebPChunkXMP.TYPE_EXIF)) {
+WebPChunk chunk = reader.readChunk();
+return chunk == null ? null : new
WebPImageMetadata((TiffImageMetadata) new
TiffImageParser().getMetadata(chunk.getBytes()));
+}
+}
+
+@Override
+public String getXmpXml(ByteSource byteSource, XmpImagingParameters
params) throws ImageReadException, IOException {
+try (ChunksReader reader =
[jira] [Closed] (POOL-408) A typo of KeyedPooledObjectFactory on the site and Javadoc
[
https://issues.apache.org/jira/browse/POOL-408?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Phil Steitz closed POOL-408.
> A typo of KeyedPooledObjectFactory on the site and Javadoc
> --
>
> Key: POOL-408
> URL: https://issues.apache.org/jira/browse/POOL-408
> Project: Commons Pool
> Issue Type: Bug
>Reporter: Zhenyu Luo
>Priority: Major
> Fix For: 2.12.0
>
> Attachments: screenshot-20220727-173624.png
>
>
> On the homepage, the interface *KeyedPoolableObjectFactory* does not exist.
> The correct interface is {*}KeyedPooledObjectFactory{*}.
> https://commons.apache.org/proper/commons-pool/
> !screenshot-20220727-173624.png!
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Closed] (POOL-409) BasicDataSource should support GenericObjectPool->getStatsString()
[
https://issues.apache.org/jira/browse/POOL-409?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Phil Steitz closed POOL-409.
> BasicDataSource should support GenericObjectPool->getStatsString()
> --
>
> Key: POOL-409
> URL: https://issues.apache.org/jira/browse/POOL-409
> Project: Commons Pool
> Issue Type: Improvement
>Affects Versions: 2.11.1
>Reporter: Thomas Freller
>Priority: Major
> Labels: improvement
> Fix For: 2.12.0
>
> Original Estimate: 2h
> Remaining Estimate: 2h
>
> Hello,
> I'm developing a Application that is running on a default JRE without an
> Webserver/JMX.
> For optimizing Database connections it would be very useful if i could access
>
> BasicDataSource->GenericObjectPool->{*}getStatsString(){*}
>
> I don't see any reason why this Method is protected and not public in
> GenericObjectPool.
> Then BasicDataSource shoud provide a method getStatsString() or the values
> that represent the statistic data.
>
> If there is any other easy way to access this data within my Java Code I'll
> implement this if you could give me an example how to get this working
> easily. I don't want do configure any jmx stuff.
>
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342192726
##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -0,0 +1,351 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.webp;
+
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.ImageInfo;
+import org.apache.commons.imaging.ImageParser;
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.common.XmpEmbeddable;
+import org.apache.commons.imaging.common.XmpImagingParameters;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffImageParser;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunk;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANIM;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANMF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkEXIF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkICCP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8L;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8X;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXMP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXYZW;
+
+import java.awt.Dimension;
+import java.awt.image.BufferedImage;
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import static org.apache.commons.imaging.common.BinaryFunctions.read4Bytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.readBytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.skipBytes;
+
+public class WebPImageParser extends ImageParser
implements XmpEmbeddable {
+
+private static final String DEFAULT_EXTENSION =
ImageFormats.WEBP.getDefaultExtension();
+private static final String[] ACCEPTED_EXTENSIONS =
ImageFormats.WEBP.getExtensions();
+
+@Override
+public WebPImagingParameters getDefaultParameters() {
+return new WebPImagingParameters();
+}
+
+@Override
+public String getName() {
+return "WebP-Custom";
+}
+
+@Override
+public String getDefaultExtension() {
+return DEFAULT_EXTENSION;
+}
+
+@Override
+protected String[] getAcceptedExtensions() {
+return ACCEPTED_EXTENSIONS;
+}
+
+@Override
+protected ImageFormat[] getAcceptedTypes() {
+return new ImageFormat[]{ImageFormats.WEBP};
+}
+
+static int readFileHeader(InputStream is) throws IOException,
ImageReadException {
+byte[] buffer = new byte[4];
+if (is.read(buffer) < 4 ||
!WebPConstants.RIFF_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");
+}
+
+int fileSize = read4Bytes("File Size", is, "Not a Valid WebP File",
ByteOrder.LITTLE_ENDIAN);
+if (fileSize < 0) {
+throw new ImageReadException("File Size is too long:" + fileSize);
+}
+
+if (is.read(buffer) < 4 ||
!WebPConstants.WEBP_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");
+}
+
+return fileSize;
+}
+
+@Override
+public WebPImageMetadata getMetadata(ByteSource byteSource,
WebPImagingParameters params) throws ImageReadException, IOException {
+try (ChunksReader reader = new ChunksReader(byteSource,
WebPChunkXMP.TYPE_EXIF)) {
+WebPChunk chunk = reader.readChunk();
+return chunk == null ? null : new
WebPImageMetadata((TiffImageMetadata) new
TiffImageParser().getMetadata(chunk.getBytes()));
+}
+}
+
+@Override
+public String getXmpXml(ByteSource byteSource, XmpImagingParameters
params) throws ImageReadException, IOException {
+try (ChunksReader reader =
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342192570
##
src/main/java/org/apache/commons/imaging/internal/SafeOperations.java:
##
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.internal;
+
+import java.util.Arrays;
+
+/**
+ * Provides safe arithmetic operations to avoid, for example,
+ * numeric overflows.
+ *
+ * @since 1.0-alpha4
+ */
+public class SafeOperations {
+
+/**
+ * Applies {@link Math#addExact(int, int)} to a variable
+ * length array of integers.
+ *
+ * @param values variable length array of integers.
+ * @return the values safely added.
+ */
+public static int add(int... values) {
+if (values == null || values.length < 2) {
+throw new IllegalArgumentException("You must provide at least two
elements to be added");
+}
+return Arrays
Review Comment:
> I didn't think too much about this class yet. Wrote the first version that
came to my mind without worrying about optimizing it. Maybe we can check if
that becomes an issue and fix it if needed?
Okay. It's really not that noticeable compared to the I/O overhead, so we
don't have to rush to deal with it
##
src/main/java/org/apache/commons/imaging/internal/SafeOperations.java:
##
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.internal;
+
+import java.util.Arrays;
+
+/**
+ * Provides safe arithmetic operations to avoid, for example,
+ * numeric overflows.
+ *
+ * @since 1.0-alpha4
+ */
+public class SafeOperations {
+
+/**
+ * Applies {@link Math#addExact(int, int)} to a variable
+ * length array of integers.
+ *
+ * @param values variable length array of integers.
+ * @return the values safely added.
+ */
+public static int add(int... values) {
+if (values == null || values.length < 2) {
+throw new IllegalArgumentException("You must provide at least two
elements to be added");
+}
+return Arrays
Review Comment:
> I didn't think too much about this class yet. Wrote the first version that
came to my mind without worrying about optimizing it. Maybe we can check if
that becomes an issue and fix it if needed?
Okay. It's really not that noticeable compared to the I/O overhead, so we
don't have to rush to deal with it.
--
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-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[jira] [Closed] (POOL-402) Check blockWhenExhausted in hasBorrowWaiters
[ https://issues.apache.org/jira/browse/POOL-402?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Phil Steitz closed POOL-402. > Check blockWhenExhausted in hasBorrowWaiters > > > Key: POOL-402 > URL: https://issues.apache.org/jira/browse/POOL-402 > Project: Commons Pool > Issue Type: Improvement >Affects Versions: 2.11.1 >Reporter: Bruno P. Kinoshita >Priority: Major > Fix For: 2.12.0 > > Time Spent: 2h 10m > Remaining Estimate: 0h > > Placeholder for https://github.com/apache/commons-pool/pull/116 -- This message was sent by Atlassian Jira (v8.20.10#820010)
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342192570
##
src/main/java/org/apache/commons/imaging/internal/SafeOperations.java:
##
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.internal;
+
+import java.util.Arrays;
+
+/**
+ * Provides safe arithmetic operations to avoid, for example,
+ * numeric overflows.
+ *
+ * @since 1.0-alpha4
+ */
+public class SafeOperations {
+
+/**
+ * Applies {@link Math#addExact(int, int)} to a variable
+ * length array of integers.
+ *
+ * @param values variable length array of integers.
+ * @return the values safely added.
+ */
+public static int add(int... values) {
+if (values == null || values.length < 2) {
+throw new IllegalArgumentException("You must provide at least two
elements to be added");
+}
+return Arrays
Review Comment:
> I didn't think too much about this class yet. Wrote the first version that
came to my mind without worrying about optimizing it. Maybe we can check if
that becomes an issue and fix it if needed?
Maybe. It's really not that noticeable compared to the I/O overhead.
--
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-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[jira] [Closed] (POOL-394) GenericKeyedObjectPool doesn't have getKeys() implemented
[ https://issues.apache.org/jira/browse/POOL-394?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Phil Steitz closed POOL-394. > GenericKeyedObjectPool doesn't have getKeys() implemented > - > > Key: POOL-394 > URL: https://issues.apache.org/jira/browse/POOL-394 > Project: Commons Pool > Issue Type: Bug >Affects Versions: 2.4.2 >Reporter: Vamsi Pavan Kumar Sanka >Priority: Major > Fix For: 2.12.0 > > > getKeys() not really implemented as specified here: > [https://commons.apache.org/proper/commons-pool2/apidocs/org/apache/commons/pool2/impl/GenericKeyedObjectPool.html#getKeys(]) > This API is needed in various scenarios. Current Jira is to expose this API. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Closed] (POOL-405) NullPointerException at org.apache.commons.pool2.impl.GenericKeyedObjectPool.invalidateObject(GenericKeyedObjectPool.java:1343)
[
https://issues.apache.org/jira/browse/POOL-405?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Phil Steitz closed POOL-405.
> NullPointerException at
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.invalidateObject(GenericKeyedObjectPool.java:1343)
> ---
>
> Key: POOL-405
> URL: https://issues.apache.org/jira/browse/POOL-405
> Project: Commons Pool
> Issue Type: Bug
>Reporter: Gary D. Gregory
>Assignee: Gary D. Gregory
>Priority: Major
> Fix For: 2.12.0
>
>
> You get a {{NullPointerException}} at
> {{org.apache.commons.pool2.impl.GenericKeyedObjectPool.invalidateObject(GenericKeyedObjectPool.java:1343)}}
> when you pass an unknown key.
> The exception should be an {{IllegalStateException}} instead.
> For example:
>
> {noformat}
> java.lang.NullPointerException
> at
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.invalidateObject(GenericKeyedObjectPool.java:1343)
> at
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.invalidateObject(GenericKeyedObjectPool.java:1320)
> {noformat}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Closed] (POOL-393) BaseGenericObjectPool.jmxRegister may cost too much time
[
https://issues.apache.org/jira/browse/POOL-393?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Phil Steitz closed POOL-393.
> BaseGenericObjectPool.jmxRegister may cost too much time
>
>
> Key: POOL-393
> URL: https://issues.apache.org/jira/browse/POOL-393
> Project: Commons Pool
> Issue Type: Improvement
>Affects Versions: 2.4.2
>Reporter: Shichao Yuan
>Priority: Major
> Fix For: 2.12.0
>
> Time Spent: 2h 40m
> Remaining Estimate: 0h
>
>
> When creating many pools, I find that it tasks too much time to register jmx.
> In the code, the ObjectName's postfix always starts with 1, so many
> InstanceAlreadyExistsExceptions may be thrown before registered successfully.
> Maybe a random number is a better choice, or a atomic long.
> {quote}private ObjectName jmxRegister(BaseObjectPoolConfig config,
> String jmxNameBase, String jmxNamePrefix) {
> ObjectName objectName = null;
> MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
> int i = 1;
> boolean registered = false;
> String base = config.getJmxNameBase();
> if (base == null)
> Unknown macro: \{ base = jmxNameBase; }
> while (!registered) {
> try {
> ObjectName objName;
> // Skip the numeric suffix for the first pool in case there is
> // only one so the names are cleaner.
> if (i == 1)
> Unknown macro: \{ objName = new ObjectName(base + jmxNamePrefix); }
> else
> Unknown macro: \{ objName = new ObjectName(base + jmxNamePrefix + i); }
> mbs.registerMBean(this, objName);
> objectName = objName;
> registered = true;
> } catch (MalformedObjectNameException e) {
> if (BaseObjectPoolConfig.DEFAULT_JMX_NAME_PREFIX.equals(
> jmxNamePrefix) && jmxNameBase.equals(base))
> Unknown macro: \{ // Shouldn't happen. Skip registration if it does.
> registered = true; }
> else
> Unknown macro: \{ // Must be an invalid name. Use the defaults instead.
> jmxNamePrefix = BaseObjectPoolConfig.DEFAULT_JMX_NAME_PREFIX; base =
> jmxNameBase; }
> } catch (InstanceAlreadyExistsException e)
> Unknown macro: \{ // Increment the index and try again i++; }
> catch (MBeanRegistrationException e)
> Unknown macro: \{ // Shouldn't happen. Skip registration if it does.
> registered = true; }
> catch (NotCompliantMBeanException e)
> }
> return objectName;
> }
> {quote}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Closed] (POOL-391) GenericKeyedObjectPool is not thread safe when invoke method `borrowObject` and `destroy` simultaneously
[
https://issues.apache.org/jira/browse/POOL-391?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Phil Steitz closed POOL-391.
> GenericKeyedObjectPool is not thread safe when invoke method `borrowObject`
> and `destroy` simultaneously
> -
>
> Key: POOL-391
> URL: https://issues.apache.org/jira/browse/POOL-391
> Project: Commons Pool
> Issue Type: Bug
>Affects Versions: 2.4.2, 2.5.0, 2.6.0, 2.7.0, 2.8.0, 2.9.0
>Reporter: Codievilky August
>Priority: Blocker
> Fix For: 2.12.0
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> The method `brrowObject` is not thread safe with `destroy` or `clear` method.
> The reason is when use GenericKeyedObjectPool#destroy method，it did not
> ensure the *Atomicity* of destroy object from the ObjectDeque。
> This may cause in the GenericKeyedObjectPool#borrowObject method，may get the
> wrong number of GenericKeyedObjectPool.ObjectDeque#getCreateCount when need
> to create. And the GenericKeyedObjectPool#borrowObject will block until
> timeout.
> Here is the sample test code to recur the bug:
> {code:java}
> // code placeholder
> public class CommonPoolMultiThreadTest {
> public static void main(String[] args) {
> GenericKeyedObjectPoolConfig config = new GenericKeyedObjectPoolConfig();
> config.setMaxTotalPerKey(1);
> config.setMinIdlePerKey(0);
> config.setMaxIdlePerKey(-1);
> config.setMaxTotal(-1);
> config.setMaxWaitMillis(TimeUnit.SECONDS.toMillis(5));
> GenericKeyedObjectPool testPool = new
> GenericKeyedObjectPool<>(
> new KeyedPooledObjectFactory() {
> @Override
> public PooledObject makeObject(Integer key) throws
> Exception {
> System.out.println("start to create");
> return new DefaultPooledObject<>(10);
> } @Override
> public void destroyObject(Integer key, PooledObject p)
> throws Exception {
> System.out.println("start to destroy");
> Thread.sleep(2000);
> } @Override
> public boolean validateObject(Integer key, PooledObject p)
> {
> return true;
> } @Override
> public void activateObject(Integer key, PooledObject p)
> throws Exception {
> // do nothing
> } @Override
> public void passivateObject(Integer key, PooledObject p)
> throws Exception {
> // do nothing
> }
> }, config
> );
> int borrowKey = 10;
> Thread t = new Thread(() -> {
> try {
> while (true) {
> Integer integer = testPool.borrowObject(borrowKey);
> testPool.returnObject(borrowKey, integer);
> Thread.sleep(10);
> }
> } catch (Exception e) {
> e.printStackTrace();
> System.exit(1);
> }
> });
> Thread t2 = new Thread(() -> {
> try {
> while (true) {
> testPool.clear(borrowKey);
> Thread.sleep(10);
> }
> } catch (Exception e) {
> e.printStackTrace();
> System.exit(1);
> }
> });
> t.start();
> t2.start();
> }
> }
> {code}
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
kinow commented on code in PR #254: URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342192494 ## src/main/java/org/apache/commons/imaging/formats/webp/WebPImageMetadata.java: ## @@ -0,0 +1,53 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.imaging.formats.webp; + +import org.apache.commons.imaging.common.GenericImageMetadata; +import org.apache.commons.imaging.formats.tiff.TiffImageMetadata; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +/** + * @since 1.0-alpha4 Review Comment: 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. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342192476
##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -0,0 +1,326 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.webp;
+
+import org.apache.commons.imaging.AbstractImageParser;
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.ImageInfo;
+import org.apache.commons.imaging.ImagingException;
+import org.apache.commons.imaging.bytesource.ByteSource;
+import org.apache.commons.imaging.common.XmpEmbeddable;
+import org.apache.commons.imaging.common.XmpImagingParameters;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffImageParser;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunk;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8L;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8X;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXMP;
+
+import java.awt.Dimension;
+import java.awt.image.BufferedImage;
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import static org.apache.commons.imaging.common.BinaryFunctions.read4Bytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.readBytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.skipBytes;
+
+/**
+ * @since 1.0-alpha4
+ */
+public class WebPImageParser extends
AbstractImageParser implements
XmpEmbeddable {
+
+private static final String DEFAULT_EXTENSION =
ImageFormats.WEBP.getDefaultExtension();
+private static final String[] ACCEPTED_EXTENSIONS =
ImageFormats.WEBP.getExtensions();
+
+@Override
+public WebPImagingParameters getDefaultParameters() {
+return new WebPImagingParameters();
+}
+
+@Override
+public String getName() {
+return "WebP-Custom";
+}
+
+@Override
+public String getDefaultExtension() {
+return DEFAULT_EXTENSION;
+}
+
+@Override
+protected String[] getAcceptedExtensions() {
+return ACCEPTED_EXTENSIONS;
+}
+
+@Override
+protected ImageFormat[] getAcceptedTypes() {
+return new ImageFormat[]{ImageFormats.WEBP};
+}
+
+/**
+ * Read the file header of WebP file.
+ *
+ * @return file size in file header (including the WebP signature,
+ * excluding the TIFF signature and the file size field).
+ */
+private static int readFileHeader(InputStream is) throws IOException,
ImagingException {
+byte[] buffer = new byte[4];
+if (is.read(buffer) < 4 ||
!WebPConstants.RIFF_SIGNATURE.equals(buffer)) {
+throw new ImagingException("Not a Valid WebP File");
+}
+
+int fileSize = read4Bytes("File Size", is, "Not a Valid WebP File",
ByteOrder.LITTLE_ENDIAN);
+if (fileSize < 0) {
+throw new ImagingException("File Size is too long:" + fileSize);
+}
+
+if (is.read(buffer) < 4 ||
!WebPConstants.WEBP_SIGNATURE.equals(buffer)) {
+throw new ImagingException("Not a Valid WebP File");
Review Comment:
No, maybe there was, but it's doing exact same thing as other parsers.
Resolving this comment.
##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -0,0 +1,326 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342192400
##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunk.java:
##
@@ -29,30 +30,50 @@
* used by the parser.
*
* @see https://developers.google.com/speed/webp/docs/riff_container;>WebP
Container Specification
- *
* @since 1.0-alpha4
*/
public abstract class WebPChunk extends BinaryFileParser {
private final int type;
private final int size;
protected final byte[] bytes;
+private int chunkSize;
Review Comment:
The `final` modifier should be added to it.
--
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-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342192363
##
src/test/java/org/apache/commons/imaging/formats/webp/WebPBaseTest.java:
##
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.imaging.formats.webp;
+
+import org.apache.commons.imaging.*;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.List;
+
+public abstract class WebPBaseTest extends AbstractImagingTest {
Review Comment:
No we did not. Resolving this comment.
--
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-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342192287
##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVP8X.java:
##
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.webp.chunks;
+
+import org.apache.commons.imaging.ImagingException;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+/**
+ * {@code
+ * 0 1 2 3
+ * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | |
+ * | WebP file header (12 bytes) |
+ * | |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | ChunkHeader('VP8X') |
+ * | |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |Rsv|I|L|E|X|A|R| Reserved|
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | Canvas Width Minus One | ...
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * ... Canvas Height Minus One|
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * }
+ *
+ * @see https://developers.google.com/speed/webp/docs/riff_container#extended_file_format;>Extended
File Format
+ *
+ * @since 1.0-alpha4
+ */
+public final class WebPChunkVP8X extends WebPChunk {
+private final boolean hasICC;
+private final boolean hasAlpha;
+private final boolean hasExif;
+private final boolean hasXmp;
+private final boolean isAnimation;
+private final int canvasWidth;
+private final int canvasHeight;
+
+public WebPChunkVP8X(int type, int size, byte[] bytes) throws
ImagingException {
+super(type, size, bytes);
+
+if (size != 10) {
+throw new ImagingException("VP8X chunk size must be 10");
+}
+
+int mark = bytes[0] & 0xFF;
+this.hasICC = (mark & 0b0010_) != 0;
+this.hasAlpha = (mark & 0b0001_) != 0;
+this.hasExif = (mark & 0b_1000) != 0;
+this.hasXmp = (mark & 0b_0100) != 0;
+this.isAnimation = (mark & 0b_0010) != 0;
+
+this.canvasWidth = (bytes[4] & 0xFF) + ((bytes[5] & 0xFF) << 8) +
((bytes[6] & 0xFF) << 16) + 1;
+this.canvasHeight = (bytes[7] & 0xFF) + ((bytes[8] & 0xFF) << 8) +
((bytes[9] & 0xFF) << 16) + 1;
+
+if (canvasWidth < 0 || canvasHeight < 0 || canvasWidth * canvasHeight
< 0) {
+throw new ImagingException("Illegal canvas size");
+}
+}
+
+public boolean isHasICC() {
+return hasICC;
+}
+
+public boolean isHasAlpha() {
Review Comment:
Done! Maintaining the variable as `hasAlpha`.
--
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-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342192216
##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVP8X.java:
##
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.formats.webp.chunks;
+
+import org.apache.commons.imaging.ImagingException;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+/**
+ * {@code
+ * 0 1 2 3
+ * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | |
+ * | WebP file header (12 bytes) |
+ * | |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | ChunkHeader('VP8X') |
+ * | |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |Rsv|I|L|E|X|A|R| Reserved|
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | Canvas Width Minus One | ...
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * ... Canvas Height Minus One|
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * }
+ *
+ * @see https://developers.google.com/speed/webp/docs/riff_container#extended_file_format;>Extended
File Format
+ *
+ * @since 1.0-alpha4
+ */
+public final class WebPChunkVP8X extends WebPChunk {
+private final boolean hasICC;
+private final boolean hasAlpha;
+private final boolean hasExif;
+private final boolean hasXmp;
+private final boolean isAnimation;
+private final int canvasWidth;
+private final int canvasHeight;
+
+public WebPChunkVP8X(int type, int size, byte[] bytes) throws
ImagingException {
+super(type, size, bytes);
+
+if (size != 10) {
+throw new ImagingException("VP8X chunk size must be 10");
+}
+
+int mark = bytes[0] & 0xFF;
+this.hasICC = (mark & 0b0010_) != 0;
+this.hasAlpha = (mark & 0b0001_) != 0;
+this.hasExif = (mark & 0b_1000) != 0;
+this.hasXmp = (mark & 0b_0100) != 0;
+this.isAnimation = (mark & 0b_0010) != 0;
+
+this.canvasWidth = (bytes[4] & 0xFF) + ((bytes[5] & 0xFF) << 8) +
((bytes[6] & 0xFF) << 16) + 1;
+this.canvasHeight = (bytes[7] & 0xFF) + ((bytes[8] & 0xFF) << 8) +
((bytes[9] & 0xFF) << 16) + 1;
+
+if (canvasWidth < 0 || canvasHeight < 0 || canvasWidth * canvasHeight
< 0) {
Review Comment:
Not needed, resolving it.
--
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-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342192170
##
src/main/java/org/apache/commons/imaging/internal/SafeOperations.java:
##
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.internal;
+
+import java.util.Arrays;
+
+/**
+ * Provides safe arithmetic operations to avoid, for example,
+ * numeric overflows.
+ *
+ * @since 1.0-alpha4
+ */
+public class SafeOperations {
+
+/**
+ * Applies {@link Math#addExact(int, int)} to a variable
+ * length array of integers.
+ *
+ * @param values variable length array of integers.
+ * @return the values safely added.
+ */
+public static int add(int... values) {
+if (values == null || values.length < 2) {
+throw new IllegalArgumentException("You must provide at least two
elements to be added");
+}
+return Arrays
Review Comment:
I didn't think too much about this class yet. Wrote the first version that
came to my mind without worrying about optimizing it. Maybe we can check if
that becomes an issue and fix it if needed?
--
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-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342192003
##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunk.java:
##
@@ -61,22 +74,41 @@ public String getTypeDescription() {
(byte) ((type >> 24) & 0xff)}, StandardCharsets.UTF_8);
}
+/**
+ * @return the payload size.
+ */
public int getPayloadSize() {
return size;
}
+/**
+ * @return the chunk size.
+ */
public int getChunkSize() {
// if chunk size is odd, a single padding byte is added
int padding = (size % 2) != 0 ? 1 : 0;
// Chunk FourCC (4 bytes) + Chunk Size (4 bytes) + Chunk Payload (n
bytes) + Padding
-return 4 + 4 + size + padding;
+int chunkFourCCandSize = 4 + 4;
+int paddedSize = Math.addExact(size, padding);
+return Math.addExact(chunkFourCCandSize, paddedSize);
Review Comment:
Have a look if that's what you had in mind :+1: 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-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342191581
##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -277,13 +279,14 @@ public void close() throws IOException {
WebPChunk readChunk() throws ImagingException, IOException {
while (sizeCount < fileSize) {
-int type = read4Bytes("Chunk Type", is, "Not a Valid WebP
File", ByteOrder.LITTLE_ENDIAN);
-int payloadSize = read4Bytes("Chunk Size", is, "Not a Valid
WebP File", ByteOrder.LITTLE_ENDIAN);
+int type = read4Bytes("Chunk Type", is, "Not a valid WebP
file", ByteOrder.LITTLE_ENDIAN);
+int payloadSize = read4Bytes("Chunk Size", is, "Not a valid
WebP file", ByteOrder.LITTLE_ENDIAN);
if (payloadSize < 0) {
throw new ImagingException("Chunk Payload is too long:" +
payloadSize);
}
boolean padding = (payloadSize % 2) != 0;
-int chunkSize = payloadSize + 8 + (padding ? 1 : 0);
+int extraPadding = Math.addExact(8, (padding ? 1 : 0));
Review Comment:
>The maximum value of extraPadding is 9, so there is no risk of overflow.
`9` as per specification? Or is there a constant somewhere?
Asking as we had security issues that had to be fixed (with some urgency,
which is no fun) where users crafted images with custom metadata, many times
invalid image formats.
So we just need to confirm if `9` is the maximum value, or if the user can
somehow manipulate it.
>Furthermore, the name extraPadding is somewhat misleading, since padding
only refers to the padding at the end of the chunk.
Sorry, I just changed that, pushing a new version now... it uses a function
to avoid having to delcare variables.
##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunk.java:
##
@@ -61,22 +74,41 @@ public String getTypeDescription() {
(byte) ((type >> 24) & 0xff)}, StandardCharsets.UTF_8);
}
+/**
+ * @return the payload size.
+ */
public int getPayloadSize() {
return size;
}
+/**
+ * @return the chunk size.
+ */
public int getChunkSize() {
// if chunk size is odd, a single padding byte is added
int padding = (size % 2) != 0 ? 1 : 0;
// Chunk FourCC (4 bytes) + Chunk Size (4 bytes) + Chunk Payload (n
bytes) + Padding
-return 4 + 4 + size + padding;
+int chunkFourCCandSize = 4 + 4;
+int paddedSize = Math.addExact(size, padding);
+return Math.addExact(chunkFourCCandSize, paddedSize);
Review Comment:
Good idea. Let me try that instead.
--
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-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342191274
##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVp8x.java:
##
@@ -42,76 +44,105 @@
* }
*
* @see https://developers.google.com/speed/webp/docs/riff_container#extended_file_format;>Extended
File Format
- *
* @since 1.0-alpha4
*/
-public final class WebPChunkVP8X extends WebPChunk {
-private final boolean hasICC;
-private final boolean hasAlpha;
-private final boolean hasExif;
-private final boolean hasXmp;
-private final boolean isAnimation;
+public final class WebPChunkVp8x extends WebPChunk {
+private final boolean icc;
Review Comment:
No strong opinion here, let me revert it.
--
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-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[jira] [Comment Edited] (IO-813) LastModifiedFileComparator should not throw exceptions, period
[ https://issues.apache.org/jira/browse/IO-813?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17770928#comment-17770928 ] Elliotte Rusty Harold edited comment on IO-813 at 10/1/23 9:30 PM: --- Throwing an exception is not a problem. Throwing an exception from Comparable.compareTo is a big problem, and throwing one not allowed by the contract of Comparable.compareTo is a huge problem. When LastModifiedFileComparator is used in Collections.sort or equivalent it can leave objects in inconsistent states. Comparable should not be used in scenarios where I/O errors are a possibility. You need something that's designed to include the possibility of an I/O error. Converting to a runtime exception does not solve the problem. It simply pretends the problem won't happen. was (Author: elharo): Throwing an exception is not a problem. Throwing an exception from Comparable.compareTo is a big problem, and throwing one not allowed by the contract of Comparable.compareTo is a huge problem. When LastModifiedFileComparator is used in Collections.sort or equivalent it can leave objects in inconsistent states. Comparable should not be used in scenarios where I/O errors are a possibility. You need something that's planned around the possibility if an I/O error. Converting to a runtime exception does not solve the problem. It simply pretends the problem won't happen. > LastModifiedFileComparator should not throw exceptions, period > -- > > Key: IO-813 > URL: https://issues.apache.org/jira/browse/IO-813 > Project: Commons IO > Issue Type: Bug >Reporter: Elliotte Rusty Harold >Priority: Major > > LastModifiedFileComparator is likely broken by design since it can > unexpectedly throw UncheckedIOException. This violates the contract of > Comparable.compareTo which is not documented to throw that exception. > I analyzed almost this exact case in detail here: > https://medium.com/@elharo/when-you-cant-throw-an-exception-b9f9b0db9ba4 > I'm not sure how to fix this now, but I'm, tempted to simply deprecate this > entire class. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (IO-813) LastModifiedFileComparator should not throw exceptions, period
[ https://issues.apache.org/jira/browse/IO-813?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17770928#comment-17770928 ] Elliotte Rusty Harold commented on IO-813: -- Throwing an exception is not a problem. Throwing an exception from Comparable.compareTo is a big problem, and throwing one not allowed by the contract of Comparable.compareTo is a huge problem. When LastModifiedFileComparator is used in Collections.sort or equivalent it can leave objects in inconsistent states. Comparable should not be used in scenarios where I/O errors are a possibility. You need something that's planned around the possibility if an I/O error. Converting to a runtime exception does not solve the problem. It simply pretends the problem won't happen. > LastModifiedFileComparator should not throw exceptions, period > -- > > Key: IO-813 > URL: https://issues.apache.org/jira/browse/IO-813 > Project: Commons IO > Issue Type: Bug >Reporter: Elliotte Rusty Harold >Priority: Major > > LastModifiedFileComparator is likely broken by design since it can > unexpectedly throw UncheckedIOException. This violates the contract of > Comparable.compareTo which is not documented to throw that exception. > I analyzed almost this exact case in detail here: > https://medium.com/@elharo/when-you-cant-throw-an-exception-b9f9b0db9ba4 > I'm not sure how to fix this now, but I'm, tempted to simply deprecate this > entire class. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (IO-811) Files.walk() direct and indirect callers fail to close the returned Stream
[ https://issues.apache.org/jira/browse/IO-811?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17770927#comment-17770927 ] Adam Rauch commented on IO-811: --- Also note that the StreamIterator JavaDoc is a bit misleading in that callers can't invoke close() on the returned iterator since the factory method returns Iterator not StreamIterator. > Files.walk() direct and indirect callers fail to close the returned > Stream > > > Key: IO-811 > URL: https://issues.apache.org/jira/browse/IO-811 > Project: Commons IO > Issue Type: Bug > Components: Utilities >Affects Versions: 2.13.0 > Environment: Windows 11, Eclipse Adoptium OpenJDK 17.0.8.1+1 >Reporter: Adam Rauch >Priority: Major > Fix For: 2.14.1 > > > JDK method Files.walk() clearly documents that the returned Stream must > be closed: "This method must be used within a try-with-resources statement or > similar control structure to ensure that the stream's open directories are > closed promptly after the stream's operations have completed." However, the > Commons IO methods that consume these streams fail to close them. And the > methods that pass on these streams fail to document that closing them is > required. > Problematic methods that I see: > * PathUtils.walk() lacks a documentation warning > * FilesUncheck.walk() both variants lack a documentation warning > * FileUtils.streamFiles() lacks a documentation warning > * FileUtils.listFiles() consumes a Files.walk() stream without closing it > * FileUtils.iterateFiles() claims to close the stream if the iterator is > consumed, however, in my test of consuming an iterator returned from this > method, a breakpoint set on FileTreeWalker.close() is never hit > Failing to close these streams can result in file handle leaks and other > problems, hence the importance of documenting and heading this requirement > from the JDK method. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342188466
##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -277,13 +279,14 @@ public void close() throws IOException {
WebPChunk readChunk() throws ImagingException, IOException {
while (sizeCount < fileSize) {
-int type = read4Bytes("Chunk Type", is, "Not a Valid WebP
File", ByteOrder.LITTLE_ENDIAN);
-int payloadSize = read4Bytes("Chunk Size", is, "Not a Valid
WebP File", ByteOrder.LITTLE_ENDIAN);
+int type = read4Bytes("Chunk Type", is, "Not a valid WebP
file", ByteOrder.LITTLE_ENDIAN);
+int payloadSize = read4Bytes("Chunk Size", is, "Not a valid
WebP file", ByteOrder.LITTLE_ENDIAN);
if (payloadSize < 0) {
throw new ImagingException("Chunk Payload is too long:" +
payloadSize);
}
boolean padding = (payloadSize % 2) != 0;
-int chunkSize = payloadSize + 8 + (padding ? 1 : 0);
+int extraPadding = Math.addExact(8, (padding ? 1 : 0));
Review Comment:
The maximum value of `extraPadding` is 9, so there is no risk of overflow.
Furthermore, the name `extraPadding ` is somewhat misleading, since
`padding` only refers to the padding at the end of the chunk.
##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunk.java:
##
@@ -61,22 +74,41 @@ public String getTypeDescription() {
(byte) ((type >> 24) & 0xff)}, StandardCharsets.UTF_8);
}
+/**
+ * @return the payload size.
+ */
public int getPayloadSize() {
return size;
}
+/**
+ * @return the chunk size.
+ */
public int getChunkSize() {
// if chunk size is odd, a single padding byte is added
int padding = (size % 2) != 0 ? 1 : 0;
// Chunk FourCC (4 bytes) + Chunk Size (4 bytes) + Chunk Payload (n
bytes) + Padding
-return 4 + 4 + size + padding;
+int chunkFourCCandSize = 4 + 4;
+int paddedSize = Math.addExact(size, padding);
+return Math.addExact(chunkFourCCandSize, paddedSize);
Review Comment:
I think we should check chunk size in constructor instead of here, this
method should not throw exception.
--
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-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[jira] [Commented] (IO-811) Files.walk() direct and indirect callers fail to close the returned Stream
[ https://issues.apache.org/jira/browse/IO-811?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17770926#comment-17770926 ] Adam Rauch commented on IO-811: --- [StreamIterator test PR|https://github.com/apache/commons-io/pull/490] created. This test will fail until StreamIterator.iterator() is fixed. [~ggregory] let me know if you want me to also fix the method and make it package private. > Files.walk() direct and indirect callers fail to close the returned > Stream > > > Key: IO-811 > URL: https://issues.apache.org/jira/browse/IO-811 > Project: Commons IO > Issue Type: Bug > Components: Utilities >Affects Versions: 2.13.0 > Environment: Windows 11, Eclipse Adoptium OpenJDK 17.0.8.1+1 >Reporter: Adam Rauch >Priority: Major > Fix For: 2.14.1 > > > JDK method Files.walk() clearly documents that the returned Stream must > be closed: "This method must be used within a try-with-resources statement or > similar control structure to ensure that the stream's open directories are > closed promptly after the stream's operations have completed." However, the > Commons IO methods that consume these streams fail to close them. And the > methods that pass on these streams fail to document that closing them is > required. > Problematic methods that I see: > * PathUtils.walk() lacks a documentation warning > * FilesUncheck.walk() both variants lack a documentation warning > * FileUtils.streamFiles() lacks a documentation warning > * FileUtils.listFiles() consumes a Files.walk() stream without closing it > * FileUtils.iterateFiles() claims to close the stream if the iterator is > consumed, however, in my test of consuming an iterator returned from this > method, a breakpoint set on FileTreeWalker.close() is never hit > Failing to close these streams can result in file handle leaks and other > problems, hence the importance of documenting and heading this requirement > from the JDK method. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[GitHub] [commons-io] labkey-adam opened a new pull request, #490: Add simple test for StreamIterator
labkey-adam opened a new pull request, #490: URL: https://github.com/apache/commons-io/pull/490 Note: This test will fail until `StreamIterator.iterator()` is fixed to return the wrapper class -- 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-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342187578
##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVp8x.java:
##
@@ -42,76 +44,105 @@
* }
*
* @see https://developers.google.com/speed/webp/docs/riff_container#extended_file_format;>Extended
File Format
- *
* @since 1.0-alpha4
*/
-public final class WebPChunkVP8X extends WebPChunk {
-private final boolean hasICC;
-private final boolean hasAlpha;
-private final boolean hasExif;
-private final boolean hasXmp;
-private final boolean isAnimation;
+public final class WebPChunkVp8x extends WebPChunk {
+private final boolean icc;
Review Comment:
Should these field names be renamed? Referring to `PamFileInfo`, perhaps it
would be better for us to keep the 'has' prefix in the field name.
--
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-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[jira] [Commented] (IO-811) Files.walk() direct and indirect callers fail to close the returned Stream
[ https://issues.apache.org/jira/browse/IO-811?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17770924#comment-17770924 ] Gary D. Gregory commented on IO-811: Just a mistake, the whole class is meant to be package private. You can test the class from the same package if you want to provide additional tests in a PR. > Files.walk() direct and indirect callers fail to close the returned > Stream > > > Key: IO-811 > URL: https://issues.apache.org/jira/browse/IO-811 > Project: Commons IO > Issue Type: Bug > Components: Utilities >Affects Versions: 2.13.0 > Environment: Windows 11, Eclipse Adoptium OpenJDK 17.0.8.1+1 >Reporter: Adam Rauch >Priority: Major > Fix For: 2.14.1 > > > JDK method Files.walk() clearly documents that the returned Stream must > be closed: "This method must be used within a try-with-resources statement or > similar control structure to ensure that the stream's open directories are > closed promptly after the stream's operations have completed." However, the > Commons IO methods that consume these streams fail to close them. And the > methods that pass on these streams fail to document that closing them is > required. > Problematic methods that I see: > * PathUtils.walk() lacks a documentation warning > * FilesUncheck.walk() both variants lack a documentation warning > * FileUtils.streamFiles() lacks a documentation warning > * FileUtils.listFiles() consumes a Files.walk() stream without closing it > * FileUtils.iterateFiles() claims to close the stream if the iterator is > consumed, however, in my test of consuming an iterator returned from this > method, a breakpoint set on FileTreeWalker.close() is never hit > Failing to close these streams can result in file handle leaks and other > problems, hence the importance of documenting and heading this requirement > from the JDK method. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Reopened] (IO-811) Files.walk() direct and indirect callers fail to close the returned Stream
[
https://issues.apache.org/jira/browse/IO-811?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Adam Rauch reopened IO-811:
---
FileUtils.listFiles() is now correctly closing the stream. Thanks!
FileUtils.iterateFiles(), however, does not close the stream after the iterator
is consumed. I believe the issue is that StreamIterator.iterator() does not
return a StreamIterator wrapper... instead it returns the supplied stream's
iterator:
{code:java}
public static Iterator iterator(final Stream stream) {
return new StreamIterator<>(stream).iterator;
}{code}
Therefore the wrapper's methods are never called.
Simple test code:
{code:java}
try (Stream stream = Stream.of(1, 2, 3).onClose(() ->
System.out.println("Closing!"))) {
System.out.println(stream.map(String::valueOf).collect(Collectors.joining("\n")));
}
Iterator iter = StreamIterator.iterator(Stream.of(1, 2, 3).onClose(()
-> System.out.println("Closing!")));
while (iter.hasNext()) {
System.out.println(iter.next());
}
{code}
Note that I couldn't test StreamIterator directly because the class isn't
public... is there a reason why the factory method is public but the class
itself is not?
> Files.walk() direct and indirect callers fail to close the returned
> Stream
>
>
> Key: IO-811
> URL: https://issues.apache.org/jira/browse/IO-811
> Project: Commons IO
> Issue Type: Bug
> Components: Utilities
>Affects Versions: 2.13.0
> Environment: Windows 11, Eclipse Adoptium OpenJDK 17.0.8.1+1
>Reporter: Adam Rauch
>Priority: Major
> Fix For: 2.14.1
>
>
> JDK method Files.walk() clearly documents that the returned Stream must
> be closed: "This method must be used within a try-with-resources statement or
> similar control structure to ensure that the stream's open directories are
> closed promptly after the stream's operations have completed." However, the
> Commons IO methods that consume these streams fail to close them. And the
> methods that pass on these streams fail to document that closing them is
> required.
> Problematic methods that I see:
> * PathUtils.walk() lacks a documentation warning
> * FilesUncheck.walk() both variants lack a documentation warning
> * FileUtils.streamFiles() lacks a documentation warning
> * FileUtils.listFiles() consumes a Files.walk() stream without closing it
> * FileUtils.iterateFiles() claims to close the stream if the iterator is
> consumed, however, in my test of consuming an iterator returned from this
> method, a breakpoint set on FileTreeWalker.close() is never hit
> Failing to close these streams can result in file handle leaks and other
> problems, hence the importance of documenting and heading this requirement
> from the JDK method.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [commons-numbers] aherbert commented on pull request #138: NUMBERS-205: Introduces isZero() to Addition and isOne() to Multiplication
aherbert commented on PR #138: URL: https://github.com/apache/commons-numbers/pull/138#issuecomment-1742167478 You can get a local coverage report using: ``` mvn test jacoco:report open target/site/jacoco/index.html ``` -- 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-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #254: [IMAGING-339] Basic WebP Support
kinow commented on PR #254: URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1742114767 > I probably just named them arbitrarily, so feel free to rename them if necessary. Thanks @Glavo ! -- 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-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on pull request #254: [IMAGING-339] Basic WebP Support
Glavo commented on PR #254: URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1742114264 I probably just named them arbitrarily, so feel free to rename them if necessary. -- 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-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Commented] (IO-813) LastModifiedFileComparator should not throw exceptions, period
[ https://issues.apache.org/jira/browse/IO-813?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17770878#comment-17770878 ] Gary D. Gregory commented on IO-813: Here is an unrelated example (in the abstract for IO) of why throwing an exception is good: https://lists.apache.org/thread/scvg6r3jxm1prh4gyvv03x6lx23zobs6 > LastModifiedFileComparator should not throw exceptions, period > -- > > Key: IO-813 > URL: https://issues.apache.org/jira/browse/IO-813 > Project: Commons IO > Issue Type: Bug >Reporter: Elliotte Rusty Harold >Priority: Major > > LastModifiedFileComparator is likely broken by design since it can > unexpectedly throw UncheckedIOException. This violates the contract of > Comparable.compareTo which is not documented to throw that exception. > I analyzed almost this exact case in detail here: > https://medium.com/@elharo/when-you-cant-throw-an-exception-b9f9b0db9ba4 > I'm not sure how to fix this now, but I'm, tempted to simply deprecate this > entire class. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[GitHub] [commons-io] garydgregory closed pull request #489: IO-811: Files.walk() direct and indirect callers fail to close the re…
garydgregory closed pull request #489: IO-811: Files.walk() direct and indirect callers fail to close the re… URL: https://github.com/apache/commons-io/pull/489 -- 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-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #489: IO-811: Files.walk() direct and indirect callers fail to close the re…
garydgregory commented on PR #489: URL: https://github.com/apache/commons-io/pull/489#issuecomment-1742082626 Closing, see above. Jira ticket resolved. -- 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-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-collections] garydgregory commented on pull request #402: COLLECTIONS-843: Implement Layered Bloom filter
garydgregory commented on PR #402: URL: https://github.com/apache/commons-collections/pull/402#issuecomment-1742082133 Let's make sure we let GitHub squash the commits once this is approved. -- 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-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Resolved] (IO-811) Files.walk() direct and indirect callers fail to close the returned Stream
[ https://issues.apache.org/jira/browse/IO-811?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Gary D. Gregory resolved IO-811. Fix Version/s: 2.14.1 Resolution: Fixed [~a...@labkey.com] Thank you for your PR. Please verify git master and/or a snapshot build from [https://repository.apache.org/content/repositories/snapshots/commons-io/commons-io/] > Files.walk() direct and indirect callers fail to close the returned > Stream > > > Key: IO-811 > URL: https://issues.apache.org/jira/browse/IO-811 > Project: Commons IO > Issue Type: Bug > Components: Utilities >Affects Versions: 2.13.0 > Environment: Windows 11, Eclipse Adoptium OpenJDK 17.0.8.1+1 >Reporter: Adam Rauch >Priority: Major > Fix For: 2.14.1 > > > JDK method Files.walk() clearly documents that the returned Stream must > be closed: "This method must be used within a try-with-resources statement or > similar control structure to ensure that the stream's open directories are > closed promptly after the stream's operations have completed." However, the > Commons IO methods that consume these streams fail to close them. And the > methods that pass on these streams fail to document that closing them is > required. > Problematic methods that I see: > * PathUtils.walk() lacks a documentation warning > * FilesUncheck.walk() both variants lack a documentation warning > * FileUtils.streamFiles() lacks a documentation warning > * FileUtils.listFiles() consumes a Files.walk() stream without closing it > * FileUtils.iterateFiles() claims to close the stream if the iterator is > consumed, however, in my test of consuming an iterator returned from this > method, a breakpoint set on FileTreeWalker.close() is never hit > Failing to close these streams can result in file handle leaks and other > problems, hence the importance of documenting and heading this requirement > from the JDK method. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[GitHub] [commons-io] garydgregory commented on a diff in pull request #489: IO-811: Files.walk() direct and indirect callers fail to close the re…
garydgregory commented on code in PR #489:
URL: https://github.com/apache/commons-io/pull/489#discussion_r1342131817
##
src/main/java/org/apache/commons/io/FileUtils.java:
##
@@ -1996,7 +1996,29 @@ public static Iterator iterateFiles(final File
directory, final IOFileFilt
* @since 1.2
*/
public static Iterator iterateFiles(final File directory, final
String[] extensions, final boolean recursive) {
-return Uncheck.apply(d -> streamFiles(d, recursive,
extensions).iterator(), directory);
+return Uncheck.apply(d -> {
+Stream stream = streamFiles(d, recursive, extensions);
+Iterator iter = stream.iterator();
+
+// Wrap the iterator to allow closing the stream after consumption
+return new Iterator() {
+@Override
+public boolean hasNext() {
+boolean ret = iter.hasNext();
+
+if (!ret) {
+stream.close();
+}
+
+return ret;
+}
+
+@Override
+public File next() {
+return iter.next();
+}
+};
+}, directory);
Review Comment:
No need for an anonymous class:
```
return StreamIterator.iterator(Uncheck.get(() -> streamFiles(directory,
recursive, extensions)));
```
##
src/main/java/org/apache/commons/io/FileUtils.java:
##
@@ -2238,7 +2260,11 @@ public static Collection listFiles(final File
directory, final IOFileFilte
* @return a collection of java.io.File with the matching files
*/
public static Collection listFiles(final File directory, final
String[] extensions, final boolean recursive) {
-return Uncheck.apply(d -> toList(streamFiles(d, recursive,
extensions)), directory);
+return Uncheck.apply(d -> {
Review Comment:
I've adopted a variant of this fix in git master. 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-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [commons-functor] sebbASF closed pull request #36: Bump commons-parent from 53 to 56
sebbASF closed pull request #36: Bump commons-parent from 53 to 56 URL: https://github.com/apache/commons-functor/pull/36 -- 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-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-functor] sebbASF closed pull request #37: Bump wagon-ssh from 3.5.2 to 3.5.3
sebbASF closed pull request #37: Bump wagon-ssh from 3.5.2 to 3.5.3 URL: https://github.com/apache/commons-functor/pull/37 -- 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-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-functor] sebbASF closed pull request #38: Bump maven-shade-plugin from 3.4.0 to 3.4.1
sebbASF closed pull request #38: Bump maven-shade-plugin from 3.4.0 to 3.4.1 URL: https://github.com/apache/commons-functor/pull/38 -- 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-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-ognl] sebbASF closed pull request #143: Bump org.slf4j:slf4j-api from 2.0.7 to 2.0.9
sebbASF closed pull request #143: Bump org.slf4j:slf4j-api from 2.0.7 to 2.0.9 URL: https://github.com/apache/commons-ognl/pull/143 -- 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-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-numbers] HaraldKi commented on pull request #138: NUMBERS-205: Introduces isZero() to Addition and isOne() to Multiplication
HaraldKi commented on PR #138: URL: https://github.com/apache/commons-numbers/pull/138#issuecomment-1742024204 Is it possible to get this codecov report out locally with a maven call? Otherwise I would push an update and hope for the best. -- 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-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-collections] Claudenw commented on pull request #402: COLLECTIONS-843: Implement Layered Bloom filter
Claudenw commented on PR #402: URL: https://github.com/apache/commons-collections/pull/402#issuecomment-1741991305 @aherbert thank you for your review. Changes have been made to the base code. One of these days I will figure out how you see all those things that I seem to miss. -- 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-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
