Skip to content

Json.NET throws exception while deserializing relative Uri #629

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

Closed
megri opened this issue Apr 26, 2014 · 5 comments
Closed

Json.NET throws exception while deserializing relative Uri #629

megri opened this issue Apr 26, 2014 · 5 comments
Milestone

Comments

@megri
Copy link

megri commented Apr 26, 2014

This bug was introduced with commit df2bfc9.

The problem is that the Uri is created without a UriKind, thus implicitly using UriKind.Absolute, which throws an exception for relative Uris.

Is there a compelling reason to replace the standard [non-buggy] serializer?

@Mpdreamz Mpdreamz added the Bug label May 2, 2014
@Mpdreamz
Copy link
Member

Mpdreamz commented May 2, 2014

The reason was that over the years i've had many people surprised by the fact that Uri properties serialise as objects by default when 9/10 times they want to serialise it as string and deserialise it Uri.

So build in support to do that instead of having folks write their own json converter made sense.

I could make it the Uri serialization optin/optout through ConnectionSettings. My gut says it should be an opt-out, any thoughts on this?

Ping @crunchie84 for his thoughts as well since he wrote the PR for it initially.

@crunchie84
Copy link

My initial PR was only for making the Uri <=> String json conversion easier. If this is a headache for the majority of uses of Nest i would recommend setting it default to serialize Uri's to string.

Since 1.0.0 release of Nest introduces a lot of other breaking changes for the better this seems the moment to do so. People can still override the (new) default serialization behaviour by implementing AddContractJsonConverters() @Mpdreamz ?

@Mpdreamz Mpdreamz added this to the NEST 1.0-beta2 milestone May 5, 2014
@megri
Copy link
Author

megri commented Jul 23, 2014

@Mpdreamz Uris are serialized to strings since at least Newtonsoft.Json 5.0.8, the oldest version I currently have available.

class Foo {
    public Uri X { get; set; }
}

JsonConvert.SerializeObject(new Foo { X = new Uri("http://www.google.com") });

serializes to {"X":"http://www.google.com/"} by the vanilla implementation

@Mpdreamz
Copy link
Member

Ok will investigate further, at the time of this commit we were on 6.0.1 so i wonder what is causing the difference in observed behavior.

https://github.com/elasticsearch/elasticsearch-net/blob/df2bfc99db9862db53f4ca6012ad7b37d57ade7e/src/Nest/packages.config#L3

gmarz added a commit that referenced this issue Jan 6, 2015
By removing the custom UriJsonConverter which was put in place to
serialize uris to strings by default, however Json.NET already does
this out of the box.

Closes #629
@gmarz
Copy link
Contributor

gmarz commented Jan 6, 2015

I've confirmed that Json.NET does indeed serialize Uri -> string out of the box and have proposed the above PR to remove UriJsonConverter.

I don't doubt that there was probably once a good reason for it, but as it stands now I can't reproduce a case where a Uri is serialized to an object.

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

No branches or pull requests

4 participants