SWIFT-1126 / SWIFT-1137 Add legacy extended JSON parsing, fix Date-related crashes #64
Conversation
|
|
||
| base64Str = b64Str | ||
| subtype = s | ||
| case let .string(base64): |
There was a problem hiding this comment.
this parses the following format:
{
"$binary": <base64 string>,
"$type": <hex string or number>
}
| @@ -17,8 +17,15 @@ public class ExtendedJSONDecoder { | |||
| }() | |||
|
|
|||
| /// A set of all the possible extendedJSON wrapper keys. | |||
| /// This does not include the legacy extended JSON wrapper keys. | |||
There was a problem hiding this comment.
Since we allow things like "$type" to be in a regular document (e.g. as part of a query operator), the legacy wrapper keys can't be used to determine errors like we do with the reserved ones, so we need to separate them out.
| @@ -62,6 +62,25 @@ public func sortedEqual(_ expectedValue: BSONDocument?) -> Predicate<BSONDocumen | |||
| } | |||
| } | |||
|
|
|||
| public func sortedEqual(_ expectedValue: BSON?) -> Predicate<BSON> { | |||
There was a problem hiding this comment.
This is just a copy/paste that extends this matcher to BSON
| Self.jsonTest(json: invalidStringNumberSubtype, expectation: .error()) | ||
| } | ||
|
|
||
| func testLegacyExtendedJSONDate() throws { |
There was a problem hiding this comment.
these test cases are taken from libbson
| ) | ||
|
|
||
| // don't invalidate a "$regex" query operator stored in JSON | ||
| Self.jsonTest( |
There was a problem hiding this comment.
the first of these 3 were taken from libbson too, except libbson would return an error. That seemed problematic to me, since these could easily be valid $regex query operators, just stored in JSON. I figured no longer throwing an error in these cases would not really constitute a breaking change, and that the legacy parsing should defer to the non-legacy path if it ever is ambiguous.
| self = BSONRegularExpression(pattern: patternStr, options: optionsStr) | ||
| return | ||
| } else { | ||
| // legacy / v1 extended JSON |
There was a problem hiding this comment.
this parses the following format:
{
"$regex": <string>,
"$options": <string>,
}
which is unfortunately, the exact same format as a query operator.
| @@ -48,6 +51,15 @@ extension Date: BSONValue { | |||
| ) | |||
| } | |||
| self = date | |||
| case let .number(ms): | |||
There was a problem hiding this comment.
legacy allows for a regular number instead of a $numberLong wrapper.
| internal var msSinceEpoch: Int64 { Int64((self.timeIntervalSince1970 * 1000.0).rounded()) } | ||
| /// If the date is further in the future than Int64.max milliseconds from the epoch, | ||
| /// Int64.max is returned to prevent a crash. | ||
| internal var msSinceEpoch: Int64 { |
There was a problem hiding this comment.
perhaps we should mention this behavior somewhere more user-facing, maybe on the BSON.datetime case?
also, given that edit, ah I see why we can't do that from the tests.msSinceEpoch is only used internally, did you consider an alternative approach where we throw here instead?
There was a problem hiding this comment.
good idea, done
| let regex = BSONRegularExpression(pattern: "abc", options: "ix") | ||
|
|
||
| Self.jsonTest( | ||
| json: ["val": ["$regex": "abc", "$options": "ix"]], |
There was a problem hiding this comment.
is it problematic that this query operator now gets parsed as a regex?
There was a problem hiding this comment.
It's not ideal, but this is the existing behavior in libbson, so I think we need to preserve it.
SWIFT-1126 / SWIFT-1137
This PR adds support for legacy extended JSON v1 parsing, which the old libbson-based BSON library had support for. This PR also fixes potential crashes that may occur when trying to use
Datevalues extremely far in the future (>Int64.maxms from epoch) or the past (<Int64.minms before epoch).