squid:S4929 ("'read(byte[],int,int)' should be overridden") false-positive on FilterInputStream

java
sonarlint
(Philipp Kapfer) #1

I’m using SonarLint for Eclipse version 4.1.0.201901311043 in connected mode with SonarQube 7.6 and SonarJava 5.12.1.

Rule squid:S4929 explains that when implementing InputStream or FilterInputStream, the read(byte[],int,int) method should be overriden, because the default InputStream implementation simply delegates to read(), which in turn reads byte by byte and is slow. This is a very good consideration for implementing InputStream. However, consider the following code:

public class ProcessReturnCodeAwareInputStream
		extends FilterInputStream {

	private final Process proc;

	protected ProcessReturnCodeAwareInputStream(Process proc) {
		super(proc.getInputStream());
		this.proc = proc;
	}

	@Override
	public void close() throws IOException {
		super.close();
		try {
			int exitCode = proc.waitFor();
			if (exitCode != 0) {
				throw new IOException("Process returned error status: " + exitCode);
			}
		} catch (InterruptedException e) {
			Thread.currentThread().interrupt();
			throw new IOException("Waiting for process was interrupted", e);
		}
	}
}

How would I implement read(byte[],int,int) in my FilterInputStream in a way that satisfies the rule? The FilterInputStream class simply delegates this method to its parent InputStream and doesn’t read anything itself. The performance of the filter therefore is entirely dependent on the wrapped stream. If I had performance issues because of a bad read() implementation, I would wrap it with a BufferedInputStream before filtering, because I would implement such a batch read in the filter the same way.

Can someone please explain how this rule applies to FilterInputStreams and what the proposed solution is? The Compliant Solution in the rule description simply delegates to the underlying stream, which a FilterInputStream does by default.

(Tibor Blenessy) #2

Hello @KPhi and welcome to SonarSource community!

You are right that in your example it probably doesn’t make sense to overwrite read(byte[], int, int) as your implementation is not processing the data from the stream. In such a case, you can just treat the warning as false positive.

As you point out, by default FilterInputStream just delegates to InputStream implementation, which is potentially inefficient, and this is what rule is trying to detect. The canonical example of how to properly implement this method could be BufferedInputStream from JDK itself.

I hope this helps.