-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CSHARP-3355: Add more spec tests for invalid relaxed UUIDs. #452
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
Conversation
src/MongoDB.Bson/IO/JsonReader.cs
Outdated
| { | ||
| throw new FormatException($"Invalid $uuid hyphens format: '{uuidTokenStringValue}'."); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can replace lines 2093-2112 with:
var guid = Guid.Parse(uuidToken.StringValue);
var binaryData = new BsonBinaryData(guid, GuidRepresentation.Standard);
Guid.Parse already does all the validations we need.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, done
|
Removed James since Robert picked it up. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good. Verified that:
- BsonBinarySubType is still set correctly.
- Same exception type (FormatException) is thrown as before.
- Tests run successfully.
One minor change required. You have an extraneous using statement.
src/MongoDB.Bson/IO/JsonReader.cs
Outdated
| using System.Globalization; | ||
| using System.IO; | ||
| using System.Text.RegularExpressions; | ||
| using System.Linq; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
cb5295b to
636be2f
Compare
636be2f to
2c0e72b
Compare
EG: https://evergreen.mongodb.com/version/601d956b3e8e86586c978855