-
Notifications
You must be signed in to change notification settings - Fork 354
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
fix for https://github.com/aspnet/AspNetWebStack/issues/173 #176
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fell through the cracks, sorry.
@@ -187,7 +187,7 @@ public virtual Task<HttpResponseMessage> CreateResponseMessageAsync(IList<HttpRe | |||
foreach (HttpContent httpContent in streamProvider.Contents) | |||
{ | |||
cancellationToken.ThrowIfCancellationRequested(); | |||
HttpRequestMessage innerRequest = await httpContent.ReadAsHttpRequestMessageAsync(); | |||
HttpRequestMessage innerRequest = await httpContent.ReadAsHttpRequestMessageAsync(request.RequestUri.Scheme); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid changes to unrelated tests, please make this null
-conditional e.g. request.RequestUri?.Scheme
. If the repo disallows new-ish C# features, use a ternary expression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done using a ternary. It's C# 5 so no ?.
Content = new MultipartContent("mixed") | ||
{ | ||
new HttpMessageContent(new HttpRequestMessage(HttpMethod.Get, "http://example.com/")), | ||
new HttpMessageContent(new HttpRequestMessage(HttpMethod.Post, "https://example.com/values")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a part using a path-only relative URI e.g. /api/values
since those are recommended and seem to be more common.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't supply a relative Uri within a batch request. If I do so then ParseBatchRequestsAsync calls ReadAsMultipartAsync (an extension on HttpContent) and that will throw the following error.
Message: System.IO.IOException : Error reading MIME multipart body part.
---- System.InvalidOperationException : This operation is not supported for a relative URI.
Does this suggest that its absolute uri's by design or is this a different issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Odd. But, if that's what enforced, so be it. On the off chance this is a bug I should open separately, please paste the full stack trace into this PR.
This change will use the uri scheme of the outer request for all inner requests.