[MC-122085] Generating server icon leaks encoded data buffer

See the function MinecraftServer#applyServerIconToResponse, in particular this section:

ByteBuf bytebuf = Unpooled.buffer();

try
{
	BufferedImage bufferedimage = ImageIO.read(file1);
	Validate.validState(bufferedimage.getWidth() == 64, "Must be 64 pixels wide");
	Validate.validState(bufferedimage.getHeight() == 64, "Must be 64 pixels high");
	ImageIO.write(bufferedimage, "PNG", new ByteBufOutputStream(bytebuf));
	ByteBuf bytebuf1 = Base64.encode(bytebuf);
	response.setFavicon("data:image/png;base64," + bytebuf1.toString(StandardCharsets.UTF_8));
}
catch (Exception exception)
{
	LOGGER.error("Couldn't load server icon", (Throwable)exception);
}
finally
{
	bytebuf.release();
}

While the original bytebuf is released, bytebuf1 is not.

Linked issues

relates to 1

MC-121884 Server->Client custom payload packets can leak resources Resolved

Comments 2

pokechu22 2017-11-19T06:28:59Z

Here's the same code in 17w45a (mappingless, but since MinecraftServer's class name isn't obfuscated, it's easy to find):

ByteBuf lvt_3_1_ = Unpooled.buffer();

         try {
            BufferedImage lvt_4_1_ = ImageIO.read(lvt_2_1_);
            Validate.validState(lvt_4_1_.getWidth() == 64, "Must be 64 pixels wide", new Object[0]);
            Validate.validState(lvt_4_1_.getHeight() == 64, "Must be 64 pixels high", new Object[0]);
            ImageIO.write(lvt_4_1_, "PNG", new ByteBufOutputStream(lvt_3_1_));
            ByteBuffer lvt_5_1_ = Base64.getEncoder().encode(lvt_3_1_.nioBuffer());
            p_a_1_.a("data:image/png;base64," + StandardCharsets.UTF_8.decode(lvt_5_1_).toString());
         } catch (Exception var9) {
            h.error("Couldn't load server icon", var9);
         } finally {
            lvt_3_1_.release();
         }

(Base64 is java.util.Base64, ByteBuffer is java.nio.ByteBuffer).

I'm pretty sure that means this is fixed in 17w45a (the code was the same as 1.12.2 in 17w43b).

pokechu22 2017-11-19T07:24:34Z

Confirmed fixed in 17w45a using -Dio.netty.leakDetection.level=PARANOID - leaks are logged in 43b but not in 45a (or 46a). I'm marking this as fixed in 45b because 45a has been archived, so I can't set it anymore. Thanks for the report (even though it's already fixed)!