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

Multi-file request support #109

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dotfortun3-code
Copy link

When uploading multiple files, an InvalidOperationException would occur when the request is forwarded. Copying the content to a MemoryStream first stops the exception from occurring.

Updated ToHttpContent helper function to copy files to a MemoryStream to prevent InvalidOperationException from occuring when multiple files are sent.

Update ToHttpContent helper function to copy files to MemoryStreams to prevent InvalidOperationException from occuring when multiple files are sent in a request
@twitchax
Copy link
Owner

Hey, sorry for the delay.

Can you add a passing / failing test that exercise this case, and the counter case?

@twitchax
Copy link
Owner

Hi @dotfortun3-code, I cannot repro this issue in the test. I have added more files to the CanProxyControllerPostWithFormAndFilesRequest test. Is there any way you can provide more context, or add a test that fails due to this issue?

@walterancona
Copy link

try to create an api with a multiform/form-data body in post, that accepts a IFormFileCollection in the body. You'll see instantly the problem.

@twitchax
Copy link
Owner

Ok, cool. I will try to get to this. If possible, it would be extremely helpful if you could add the test that would have failed, but is fixed by this PR, so that it remains covered.

@dotfortun3-code
Copy link
Author

I can probably take a look at adding the test soon-ish. I just had a baby a few weeks ago so I've been a little tied up haha!

@twitchax
Copy link
Owner

Woohoo! Congrats!

@walterancona
Copy link

Congratulations!!! The problem is that the test passes, but in real cases the api gives back 200, but the files are not uploaded in the folder, so maybe this should be checked in the test. I've tried it but not succeeded.

@CosimoPiscopo
Copy link

@dotfortun3-code Congrats! I notice that the fix doesn't work with text/csv files. It seems like it doesn't read the file content. I fixed this by setting the position of the Memory Stream to the beginnig before creating the Stream Content:

ms.Seek(0, SeekOrigin.Begin);
var content = new StreamContent(ms);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants