WebDavServer icon indicating copy to clipboard operation
WebDavServer copied to clipboard

Suggestion: Invert stream ownership for PUT requests

Open faiteanu opened this issue 2 years ago • 0 comments

Hi, first of all: thanks for the nice library!

I have one concern regarding the stream handling in PutHandler.cs

https://github.com/FubarDevelopment/WebDavServer/blob/a11219eece75c5daa6dc2f7126fc705765a29920/src/FubarDev.WebDavServer/Handlers/Impl/PutHandler.cs#L117-L130

Since you require document to provide a writeable stream, that makes usage in some scenarios unnecessarily difficult. Though it works well for the implemented file storage and memory storage, which both possess writeable streams, SQLiteBlobWriteStream.cs already looks cumbersome and needs to buffer the whole stream, defeating the purpose of streaming.

My suggestion is thus to pass the input stream to the document and let it handle the further processing. For the lines mentioned above, I used

var contentLength = context.RequestHeaders.ContentLength;
if (operation == PutOperation.Create)
{
    await document.CreateAsync(data, contentLength, cancellationToken).ConfigureAwait(false);
}
else
{
    await document.WriteAsync(data, startPosition ?? 0, contentLength, cancellationToken).ConfigureAwait(false);
}

This allows to write the following code (in shortened form). Note that the input stream is passed directly to the database, without requiring to buffer the whole file.

public async Task WriteAsync(Stream stream, int startPosition, int contentLength, CancellationToken ct)
{
    using (var connection = context.Database.GetDbConnection())
    {
        await connection.OpenAsync();
        using (var command = connection.CreateCommand())
        {
            command.CommandText = "UPDATE file_data SET file=@file WHERE id=@id";
            command.Parameters.AddWithValue("id", id);
            command.Parameters.AddWithValue("file", stream);
            await command.ExecuteNonQueryAsync(ct);
        }
    }
}

Since changing the method signatures of CreateAsync and WriteAsync is significant, it would break all previously implemented clients. I leave it up to you to decide whether to go this route.

Cheers, Fabian

faiteanu avatar Sep 27 '22 13:09 faiteanu