MikaelGRA / InfluxDB.Client

InfluxDB Client for .NET
MIT License
102 stars 22 forks source link

Problem with FormUrlEncodedContent #19

Closed mkontula closed 7 years ago

mkontula commented 7 years ago

FormUrlEncodedContent uses Uri.EscapeDataString internally. This method has a hard-coded limit of 32766 characters for any param. If exceeded, it throws.

I wrote a workaround in my fork. Take a look at https://github.com/mkontula/InfluxDB.Client/blob/master/src/Vibrant.InfluxDB.Client/Http/LongFormUrlEncodedContent.cs and https://github.com/mkontula/InfluxDB.Client/blob/master/src/Vibrant.InfluxDB.Client/InfluxClient.cs for possible workarounds. I'd make a PR but thanks to different editor and style settings, my resharper formatting caused 97% rewrite of InfluxClient.cs :/

MikaelGRA commented 7 years ago

I'll include this right away. Actually I did encounter this when looking up FormUrlEncodedContent, so I don't know why I did not include this in the first place.

You must have some massive URLs. =)

MikaelGRA commented 7 years ago

Small question:

I see that you are calling: Replace( "%20", "+" ) after the value has been escaped. Presumably to make the url prettier(?)

Is there any value to this? Atleast to my unit tests, there's no difference what-so-ever.

mkontula commented 7 years ago

We do have some queries which combines like thoudsands of tag values to OR clauses. They do indeed end up being quite long. Regarding the Replace-trick, I copy-pasted the FormUrlEncodedContent directly from here: https://github.com/dotnet/corefx/blob/942a255f28c407ccb83e2c955b228e67cd1b189a/src/System.Net.Http/src/System/Net/Http/FormUrlEncodedContent.cs and added the logic to Encode method.

As far as I know, + sign is valid representation of %20 (space) in Url encoded strings. %20 would do as well, but I quess with + sign it's more readable if sometimes needed to check manually. Anyway, the big boys have used it, so we don't we?

MikaelGRA commented 7 years ago

Alright. Should be fixed now.