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

Crash when sending large POST forms. Invalid URI: The Uri string is too long. #1814

Closed
kendallb opened this issue Mar 29, 2022 · 14 comments
Closed

Comments

@kendallb
Copy link
Contributor

Describe the bug
This happens when we are attempting to send emails with large body content to customers via Mailgun:

https://documentation.mailgun.com/en/latest/api-sending.html#sending

This should be a pure POST form, and the problem appears to be related to the text and html parameters that are passed as part of the POST form. Those can be quite large when the email content itself is a decent size (in this case a customer order with a lot of products, but it could be the same for a customer newsletter).

text | Body of the message. (text version)
html | Body of the message. (HTML version)

To Reproduce
I can reproduce it at will in our test environment, but I will have to see if there is a way to build a unit test to get it to fail in a similar fashion and put that into a PR. I also plan to debug it with v106 to compare what is done differently there.

Expected behavior
Should not crash ;)

Stack trace
The problem happens right here:

https://github.com/restsharp/RestSharp/blob/dev/src/RestSharp/Request/RequestContent.cs#L162

            var formContent = new FormUrlEncodedContent(
                _request.Parameters
                    .Where(x => x.Type == ParameterType.GetOrPost)
                    .Select(x => new KeyValuePair<string, string>(x.Name!, x.Value!.ToString()!))!
            );

It crashes inside System.UriHelper.EscapeString which unfortunately is buried deep inside the System.Net.Http client code when building the FormUrlEncodedContent. So this could well be a .NET bug related to HttpClient, or perhaps it's an issue we can work around some other way (perhaps doing the encoding ourselves)? Here is the relevant final part of the stack trace:

System.UriFormatException: Invalid URI: The Uri string is too long.
at System.UriHelper.EscapeString(String input, Int32 start, Int32 end, Char[] dest, Int32& destPos, Boolean isUriString, Char force1, Char force2, Char rsvd)
at System.Uri.EscapeDataString(String stringToEscape)
at System.Net.Http.FormUrlEncodedContent.Encode(String data)
at System.Net.Http.FormUrlEncodedContent.GetContentByteArray(IEnumerable1 nameValueCollection) at System.Net.Http.FormUrlEncodedContent..ctor(IEnumerable1 nameValueCollection)
at RestSharp.RequestContent.AddPostParameters(ParametersCollection postParameters) in C:\src\git\RestSharp\src\RestSharp\Request\RequestContent.cs:line 162
at RestSharp.RequestContent.BuildContent() in C:\src\git\RestSharp\src\RestSharp\Request\RequestContent.cs:line 43
at RestSharp.RestClient.d__2.MoveNext() in C:\src\git\RestSharp\src\RestSharp\RestClient.Async.cs:line 65

Desktop (please complete the following information):

  • Windows 10 or Windows Server 2016
  • .NET Framework
  • 4.8
@kendallb kendallb added the bug label Mar 29, 2022
@kendallb
Copy link
Contributor Author

Just checked and the email in question has textBody.Length = 3012 characters and htmlBody.Length = 74319 characters.

@kendallb
Copy link
Contributor Author

This looks to be a bug in .net HttpClient. You can see more here:

https://stackoverflow.com/questions/38440631/httpclient-the-uri-string-is-too-long

I have debugged the old v106 code and in v106 it fills in all the POST parameters like this:

            http.Parameters = requestParameters
                .Where(p => p.Type == ParameterType.GetOrPost)
                .Select(p => new HttpParameter(p.Name!, p.Value))
                .ToList();

which adds them all into the HttpWebRequest message Parameters field. And clearly that does all the encoding internally but does not have the Uri string problem that HttpClient has.

In reality this REALLY needs to be fixed by Microsoft in HttpClient, but that is not really an option at the moment. I suspect the solution is to do what the top answer suggests:

// Let's assume you've got your key-value pairs organised into a nice Dictionary<string, string> called formData
var encodedItems = formData.Select(i => WebUtility.UrlEncode(i.Key) + "=" + WebUtility.UrlEncode(i.Value));
var encodedContent = new StringContent(String.Join("&", encodedItems), null, "application/x-www-form-urlencoded");

// Post away!
var response = await client.PostAsync(url, encodedContent);

I will take a stab and converting it from using FormUrlEncodedContent to just a byte body content and doing all the encoding without using Uri.EscapeString functions which HttpClient is using internally. That should then fix it at the RestSharp level and we don't be hampered by a broken HttpClient implementation.

I also checked and I do not see any way in Mailgun to submit the POST as a JSON body, so form data appears to be the only way.

@kendallb
Copy link
Contributor Author

Writing a unit test for this, and it looks like this does not crash in .net 6, so the bug must have been fixed at some point but is present in .net standard 2.1 which is used in .net 4.8? We could implement this with an #if code block, or leave it the same for all versions?

@kendallb
Copy link
Contributor Author

Ok wrote a test for it to make sure it works correctly. Can't really tests this in .NET 4.8 or .NET Standard. Let me know if you want me to change this to conditional compilation.

@stale
Copy link

stale bot commented Apr 28, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Apr 28, 2022
@repo-ranger
Copy link

repo-ranger bot commented Apr 28, 2022

⚠️ This issue has been marked wontfix and will be closed in 3 days

@alexeyzimarev
Copy link
Member

I'd say that it could be conditional for the .NET Framework, but not needed for .NET Core and .NET 5+

@kendallb
Copy link
Contributor Author

Yes, but for those on .NET 4.6 etc, without this fix your code will crash. So we need something in there to make sure it works correctly. It could certainly be conditional on the targets where it fails so at least in the code as folks eventually move off the bad versions (or if Microsoft fixes it somehow), it could be removed.

My changes do work correctly with .NET 5+ etc also, but I have no problem making it conditional.

@alexeyzimarev
Copy link
Member

What I mean is that it could be conditional (include) for .NET Standard, so all .NET Framework apps will get it with your custom code, and .NET 5+ will work as-is. The intention would be to eventually remove the custom code when .NET Framework gets OOS.

@stale
Copy link

stale bot commented Jun 13, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@alexeyzimarev
Copy link
Member

I believe it's been fixed?

@kendallb
Copy link
Contributor Author

If it was merged into develop, then yes I assume it is fixed. I have to compare my tree to what is in the main branch.

@stale
Copy link

stale bot commented Aug 13, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Aug 13, 2022
@repo-ranger
Copy link

repo-ranger bot commented Aug 13, 2022

⚠️ This issue has been marked wontfix and will be closed in 3 days

@repo-ranger repo-ranger bot closed this as completed Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants