Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Content-Length no longer set when using Refit 4 #362

Closed
jdluzen opened this issue Sep 9, 2017 · 9 comments · Fixed by #374
Closed

Content-Length no longer set when using Refit 4 #362

jdluzen opened this issue Sep 9, 2017 · 9 comments · Fixed by #374
Labels

Comments

@jdluzen
Copy link

jdluzen commented Sep 9, 2017

I am currently using 3.1.0, the Content-Length header is set on requests. After upgrading to 4.0.1, it is no longer set.

@clairernovotny
Copy link
Member

How are you making the request, what does your API look like? These headers are generally handled by the HttpContent derived types and aren't something that Refit ever manually set.

@jdluzen
Copy link
Author

jdluzen commented Sep 10, 2017

The test API I used was pretty basic, something like this:

[Post("<...>")]
public interface ITemp
{
    Task Test(<some POCO here>);
}

I'm not sure, but after searching through the code based on what I know about ASP.NET Streams, their behavior changes when Flush()ing, I suspect this might be it: 879e3d1#diff-ef1e95d8d69943f1b3f91e27c7bd5986 With the change, the Length is no longer known.

@clairernovotny
Copy link
Member

/cc @Cheesebaron, any ideas?

@Cheesebaron
Copy link
Contributor

I guess this is related to PushStreamContent, which doesn't calculate the size of the stream prior to uploading. This way you can keep streaming (pushing) chunks of data, I.e. when uploading big blobs, without loading the entire thing into memory.

PushStreamContent will return -1 for Content-Length, indicating that data will be pushed:
879e3d1#diff-e9d50c57b943fec07af2cd7e77a33371R154

StreamContent is a pull model instead, where the server you are uploading to, knows the content-length header and pulls the data based on that.

This shouldn't change anything on how you receive streamed data server side. In ASP.NET it will be something like:

using (var stream = await Request.Content.ReadAsStreamAsync())
{
    // do stuff with the stream...
}

@bennor
Copy link
Contributor

bennor commented Sep 11, 2017

@Cheesebaron, do you reckon we should maybe make the push stream behaviour something that can be opted out of using a property on the [Body] attribute?

@jdluzen
Copy link
Author

jdluzen commented Sep 11, 2017

@Cheesebaron agreed that it shouldn't change usage on the server side. Unfortunately, I am using it against a 3rd party API which requires it.

@Cheesebaron
Copy link
Contributor

@onovotny @bennor I think an opt-out or opt-in model needs to be implemented. Not sure which one is preferable. Maybe it could be as simple as using StreamContent instead of PushStreamContent when opting in/out.

@bennor
Copy link
Contributor

bennor commented Sep 12, 2017

I think opt-out is preferable. It's not that common for an API to require a content length from the client, is it?

@Cheesebaron
Copy link
Contributor

Not that I know off. I don't write my API's like that 😄

@lock lock bot added the outdated label Jun 25, 2019
@lock lock bot locked and limited conversation to collaborators Jun 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants