tmenier / Flurl

Fluent URL builder and testable HTTP client for .NET
https://flurl.dev
MIT License
4.23k stars 387 forks source link

Bug with multipart boundary field value when re-using the FlurlRequest object #807

Open lmb-djaquier opened 10 months ago

lmb-djaquier commented 10 months ago

Hello,

I found incorrect Flurl behavior when re-using a Flurlrequest object with multiple PostMultipartAsyncs.

For example, with this kind of snippet, to fetch some attachments from a famous support case manager (service now) to sync with a famous other ticketing system (jira):

var jiraAddAttachmentUrl = jiraCreateIssueResponse.Self
.AppendPathSegment("attachments")
.WithSomeNiceStuff(...)
;

foreach (var currentAttachmentToAdd in snowParentAttachmentResult.Result)
{
    using (var processSingleAttachment = myActivitySource.StartActivity("Get single attachment"))
    {

        var currentAttachmentSnowUrl = SnowRootUrl
            .AppendPathSegment("api/now/attachment")
            .WithSomeNiceStuff(...)

        Stream currentAttachmentStream;

        using (var getAttachmentFromSnow = myActivitySource.StartActivity("Get File from Service Now"))
        {
            currentAttachmentStream = await currentAttachmentSnowUrl.GetStreamAsync();
        }
        using (var addFileInJira = myActivitySource.StartActivity("Post file to jira"))
        {
            var jiraAddAttachmentResponse = await jiraAddAttachmentUrl.PostMultipartAsync(mp => mp
            .AddFile("file", currentAttachmentStream, currentAttachmentToAdd.FileName)
            );
        }
    }
}

For the first iteration in fiddler/wireshark, everything seems fine

POST <jira url>/rest/api/2/issue/1324008/attachments HTTP/1.1
Host: XXXXXXXXXX
Authorization: Basic XXXXXXXXX
X-Atlassian-Token: nocheck
Transfer-Encoding: chunked
Accept-Encoding: gzip, deflate
traceparent: 00-85995a13445359dc837ac6793c3f94d6-1939b112bcbdbeed-01
Content-Type: multipart/form-data; boundary="6553d643-ca40-434f-a980-7253ee239492"

28
--6553d643-ca40-434f-a980-7253ee239492

4D
Content-Disposition: form-data; name="file"; filename="XXXXX.xlsx"

But, when the stream is updated and we try to recycle the FlurlRequest object, boundary is not changed (still 6553d643-ca40-434f-a980-7253ee239492 ). That means the pointer is wrong and the server has no content to handle. In that case, no attachement in the ticketing system.

POST <jira url>/rest/api/2/issue/1324008/attachments HTTP/1.1
Host: XXXXXXXXXX
Authorization: Basic XXXXXXXXX
X-Atlassian-Token: nocheck
Transfer-Encoding: chunked
Accept-Encoding: gzip, deflate
traceparent: 00-85995a13445359dc837ac6793c3f94d6-2bc66332b9f05488-01
Content-Type: multipart/form-data; boundary="6553d643-ca40-434f-a980-7253ee239492"

28
--a8ffd831-4f29-4763-b834-9e76b03ce26d

59
Content-Disposition: form-data; name="file"; filename="XXXXXXXX.log"

Is that a bug or something missed on our side?

I fixed it currently with a re-instanciation of the object inside the loop.

tmenier commented 10 months ago

A FlurlRequest should never be reused. Much like the HttpRequestMessage that it effectively wraps, it's underpinned by a forward-only stream. The rules mirror those of the HttpClient stack: reuse clients heavily; never reuse requests. Your fix of creating a new one is correct.

romerod commented 9 months ago

Ah ok, explained like that it makes sense.

With the fluent interface it's actually not that visible that a request is created:

var jiraAddAttachmentUrl = jiraCreateIssueResponse.Self .AppendPathSegment("attachments"); or url.WithHeader("Accept", "text/plain")

One might be tempted to reuse a request.

Might be helpful to add this information to the documentation or maybe even throw an Exception in case of PostMultipartAsync as the sent request is not usable.