Skip to content

Conversation

JamesSinclairBiomni
Copy link
Contributor

This change will use the uri scheme of the outer request for all inner requests.

@dnfclas
Copy link

dnfclas commented Jul 5, 2018

CLA assistant check
All CLA requirements met.

@dougbu dougbu self-assigned this Jul 24, 2018
Copy link
Contributor

@dougbu dougbu left a 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.

{
cancellationToken.ThrowIfCancellationRequested();
HttpRequestMessage innerRequest = await httpContent.ReadAsHttpRequestMessageAsync();
HttpRequestMessage innerRequest = await httpContent.ReadAsHttpRequestMessageAsync(request.RequestUri.Scheme);
Copy link
Contributor

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.

Copy link
Contributor Author

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"))
Copy link
Contributor

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.

Copy link
Contributor Author

@JamesSinclairBiomni JamesSinclairBiomni Jul 25, 2018

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?

Copy link
Contributor

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.

@dougbu dougbu merged commit dcee8e6 into aspnet:master Jul 25, 2018
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.

3 participants