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

Fix problems deserializing Bson long into int #913

Merged
merged 2 commits into from
Dec 11, 2014

Conversation

dmonagle
Copy link
Contributor

@dmonagle dmonagle commented Dec 4, 2014

Adds the ability to deserialize a Bson long into a native int. This could be problematic for numbers outside the range of int but it should probably be checked by the developer if they are taking Bson input. This caused quite a headache when we have data coming from a Json api and wanted to merge with the Bson from MongoDB before deserializing.

This code previously did not work:

    int dest;

    auto jsonSource = Json(2048); // Imagine this came from a web request
    assert(jsonSource.type == Json.Type.int_); // The Json type is int

    auto source = Bson.fromJson(jsonSource); // We could do this so we can merge with a Bson object loaded from the database
    assert(source.type == Bson.Type.long_); // The Bson object now has a long

    dest.deserializeBson(source); // This caused an error as it was conversion from long to int

Adds the ability to deserialize a Bson long into a native int. This could be problematic for numbers outside the range of int but it should probably be checked by the developer if they are taking Bson input. This caused quite a headache when we have data coming from a Json api and wanted to merge with the Bson from MongoDB before deserializing.

This code previously did not work:

```D
	int dest;

	auto jsonSource = Json(2048); // Imagine this came from a web request
	assert(jsonSource.type == Json.Type.int_); // The Json type is int

	auto source = Bson.fromJson(jsonSource); // We could do this so we can merge with a Bson object loaded from the database
	assert(source.type == Bson.Type.long_); // The Bson object now has a long

	dest.deserializeBson(source); // This caused an error as there is was conversion from long to int

```
@Geod24
Copy link
Contributor

Geod24 commented Dec 5, 2014

I think you should check if the long is in an int boundary before returning a (possibly) invalid value to the user.

@dmonagle
Copy link
Contributor Author

dmonagle commented Dec 5, 2014

@Geod24 I've updated the code to add an assertion for that situation. Frankly I'm torn as to whether the serializer should be checking this. You mention returning an invalid value to the user, but realistically a developer should be using long if there is any chance that the data is going to go outside of int range. We do our range checking in the controllers, well before the point of attempting to coerce it into a native variable.

I've only patched this because it was becoming seriously problematic for us and we are already looking forward with the std_data_json library and figure that the bson library will be replaced in a similar fashion in the near(ish) future.

@s-ludwig
Copy link
Member

Throwing an exception here I think makes sense, since the developer usually otherwise has no way to do the same check (other than manually deserializing the data a second time).

s-ludwig added a commit that referenced this pull request Dec 11, 2014
Fix problems deserializing Bson long into int
@s-ludwig s-ludwig merged commit 596c4de into vibe-d:master Dec 11, 2014
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