minio / minio-dotnet

MinIO Client SDK for .NET
https://docs.min.io/docs/dotnet-client-quickstart-guide.html
Apache License 2.0
575 stars 230 forks source link

Content-Type header value is wrong in the presigned put url #1150

Open vugarli opened 3 months ago

vugarli commented 3 months ago
PresignedPutObjectArgs args = new PresignedPutObjectArgs()
.WithBucket(bucketName)
.WithObject(objectName)
.WithExpiry(60 * 60 * 24)
.WithHeaders(new Dictionary<string,string>() 
{ 
    { "Content-Type", "text/plain" },
    { "Content-Length","343434"},
    { "AAAA","BBBB"}
});

var url = await _minioClient.PresignedPutObjectAsync(args);

Using the above code I have generated this presigned put url: http://localhost:9000/landingimages/1b9cdab7-60b8-4314-8faf-3edc78ee8ae5?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=UPLOADERUPLOADER%2F20240802%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20240802T165608Z&X-Amz-Expires=86400&X-Amz-SignedHeaders=host&aaaa=BBBB&content-length=343434&content-type=Minio.DataModel.Args.PresignedPutObjectArgs&X-Amz-Signature=1973fcfb6346941f8f848ed10a8919f39338be4febc7717cbf335e4967e87131

Upon checking the content-type param in the signed URL, I have noticed that content-type header has a rather unusual value.

To create the url PresignedPutObjectAsync method calls CreateRequest extension method located in RequestExtensions.cs file:

public async Task<string> PresignedPutObjectAsync(PresignedPutObjectArgs args)
{
...
    var requestMessageBuilder = await this.CreateRequest(HttpMethod.Put, args.BucketName,
        args.ObjectName,
        args.Headers, // contentType
        Convert.ToString(args.GetType(), CultureInfo.InvariantCulture), // metaData
        Utils.ObjectToByteArray(args.RequestBody)).ConfigureAwait(false);
....
}

Below is the implementation of the CreateRequest:

internal static async Task<HttpRequestMessageBuilder> CreateRequest(this IMinioClient minioClient,
    HttpMethod method,
    string bucketName = null,
    string objectName = null,
    IDictionary<string, string> headerMap = null,
    string contentType = "application/octet-stream",
    ReadOnlyMemory<byte> body = default,
    string resourcePath = null,
    bool isBucketCreationRequest = false)
{

.....

    if (headerMap is not null)
    {
        if (headerMap.TryGetValue(messageBuilder.ContentTypeKey, out var value) && !string.IsNullOrEmpty(value))
            headerMap[messageBuilder.ContentTypeKey] = contentType;

        foreach (var entry in headerMap) messageBuilder.AddOrUpdateHeaderParameter(entry.Key, entry.Value);
    }

    return messageBuilder;
}

My best guess is that Convert.ToString(args.GetType(), CultureInfo.InvariantCulture) is wrongly passed for the argument string contentType.

Also I didn't get why CreateRequest method has contentType as a parameter. Why not try to get it from headerMap and set to default value if not found? Also, it seems like contentType overrides headerMap's Content-Type when headerMap has Content-Type set, which should be the opposite?

If maintainers agree, I can open PR

vugarli commented 3 months ago

I think the .NET SDK for the MinIO Client generates a presigned PUT URL that is incompatible with S3.

According to this, X-Amz-SignedHeaders query parameter is for the headers that used to calculate the signature. And the format should be like "Lowercase(HeaderName) + ; + Lowercase(HeaderName)".

Below are the sample query parameter with the value generated in the wrong format and the code piece that is responsible for creating it.

X-Amz-SignedHeaders=host&aaaa=BBBB&content-length=343434&content-type=Minio.DataModel.Args.PresignedPutObjectArgs&X-Amz-Signature=1973fcfb6346941f8f848ed10a8919f39338be4febc7717cbf335e4967e87131

https://github.com/minio/minio-dotnet/blob/50d4fd57ab714bca5789f816b36f82056894a309/Minio/V4Authenticator.cs#L323

To make sure, I generated the same URL with the S3 SDK for .NET and observed the correct format.

I'll be opening a PR shortly to address this.