davidmoten / subethasmtp

SubEtha SMTP is a Java library for receiving SMTP mail
Other
149 stars 40 forks source link

BdatInputStream can leak streams #111

Closed valenpo closed 11 months ago

valenpo commented 1 year ago

BdatInputStream can leak decorated streams, as close do nothing.

valenpo commented 1 year ago

@davidmoten I start working to make all Stream proper closed with try blocks. It looks like it is exist stream leaks not only on BdatInputStream

davidmoten commented 1 year ago

In short, don't. From memory, stream closure happens. A session has a single inputstream which is passed to the command classes. They should not close it because further commands will not work.

valenpo commented 1 year ago

Here is a code, where stream is closed, but it could close only top level wrapper but not internal streams that decorated and this how all decorated streams could be open. It will not be a chain close of streams.

private static byte[] readAndClose(InputStream is, int maxMessageSize)
                throws IOException, TooMuchDataException {
            ByteArrayOutputStream bytes = new ByteArrayOutputStream();
            byte[] buffer = new byte[8192];
            int n;
            try {
                while ((n = is.read(buffer)) != -1) {
                    bytes.write(buffer, 0, n);
                    if (maxMessageSize > 0 && bytes.size() > maxMessageSize) {
                        throw new TooMuchDataException("message size exceeded maximum of " + maxMessageSize + "bytes");
                    }
                }
            } finally {
                // TODO creator of stream should close it, not this method
                is.close();
            }
            return bytes.toByteArray();
        }
valenpo commented 1 year ago

In short, don't. From memory, stream closure happens. A session has a single inputstream which is passed to the command classes. They should not close it because further commands will not work.

@davidmoten Accepted. Additional thoughts, Sesssion.getInputStream() should return unclosable stream, as it is confusing and close it proper on Session.closeConnection() only. And proper close streams on try block on exact command.

Also it is possible to return BufferedInputStream on Sesssion.getInputStream().

davidmoten commented 11 months ago

we already use BufferedInputStream. Closing.