LimitedInputStream read mode handling

[yazd]

LimitedInputStream in vibe.stream.counting is used to limit the number of bytes available to readers in cases similar to when an HTTP response specifies the body content-length.

However, current implementation forbids the usage of the following pattern that uses IOMode.once:

// input is an instance of LimitedInputStream in case Content-length header is available
auto input = res.bodyReader();

// 1024 is larger than received content length
const bufferLength = exactBuffer ? res.headers["Content-length"].to!size_t() : 1024;
ubyte[] buffer = new ubyte[](bufferLength);

while (!input.empty)
{
	const chunk = input.read(buffer[], IOMode.once); // <---- IOMode.once is important
	logInfo("received: %s", buffer[0 .. chunk].assumeUTF);
}

My understanding of IOMode.once is that the stream will provide whatever data is currently available (limited by the provided buffer size of course) or will block until at least some data is available.

Current implementation of LimitedInputStream checks whether the provided buffer length exceeds the maximum allowed length to be read and errors if so without considering IOMode.

So is my understanding and my expectation of the pattern above to work correctly correct?
If so, I would change LimitedInputStream.read to:

• always check length only in IOMode.all
• provide whatever number of bytes is allowed in IOMode.immediate and IOMode.once.
• only error out when read is called with IOMode.immediate and IOMode.once when all allowed bytes were consumed previously.

Below is a runnable test case:

import vibe.core.core;
import vibe.core.net;
import vibe.http.server, vibe.http.client;
import vibe.stream.operations;
import vibe.core.log;

import std.format: format;
import std.string: assumeUTF;
import std.conv: to;

void main()
{
	/*
	when exactBuffer is true,
	the buffer into which the data is being read has
	equal length to the content-length of the response.

	when exactBuffer is false, the buffer is 1024 bytes long.

	The problem manifests when a larger buffer than the data
	available on the stream is used:
	vibe.stream.counting.LimitedInputStream errors with
	`vibe.stream.counting.LimitException@../../stream/vibe/stream/counting.d(134): Size limit reached`
	*/

	bool exactBuffer = false;

	auto server = setupServer();
	runTask(&client, server.bindAddresses[0], exactBuffer);
	runApplication();
	server.stopListening();
}

HTTPListener setupServer()
{
	auto settings = new HTTPServerSettings();
	settings.port = 0;
	settings.bindAddresses = ["127.0.0.1"];

	return listenHTTP(settings, &handler);
}

bool client(NetworkAddress addr, bool exactBuffer) nothrow
{
	try
	{
		const url = "http://%s/".format(addr.toString());

		requestHTTP(
			url,
			(scope req) {},
			(scope res)
			{
				auto input = res.bodyReader();

				// 1024 is larger than received content length
				const bufferLength = exactBuffer ? res.headers["Content-length"].to!ulong() : 1024;
				ubyte[] buffer = new ubyte[](bufferLength);

				while (!input.empty)
				{
					const chunk = input.read(buffer[], IOMode.once);
					logInfo("received: %s", buffer[0 .. chunk].assumeUTF);
				}
			}
		);

		return true;
	}
	catch (Exception ex)
	{
		logError("Exception thrown: %s", ex);

		return false;
	}
	finally
	{
		exitEventLoop();
	}
}

void handler(scope HTTPServerRequest _, scope HTTPServerResponse res)
{
	res.writeBody("my length is 15");
}

[s-ludwig]

Yes, that sounds correct! LimitedInputStream hasn't been fully adjusted when IOMode was introduced (and I think there are some more stream types, too).