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

GODRIVER-1923 Error if BSON cstrings contain null bytes (#622) #684

Merged
merged 1 commit into from
Jun 14, 2021

Conversation

iwysiu
Copy link
Contributor

@iwysiu iwysiu commented Jun 11, 2021

Because I had to make a few changes to backport it: 2aca31d

@iwysiu iwysiu requested review from kevinAlbs and benjirewis June 11, 2021 17:31
Copy link
Contributor

@benjirewis benjirewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just compared with the release/1.4 backport and saw one extra change (seems harmless). Should you mark GODRIVER-1923 with release version 1.3.8?

mongo/mongo_test.go Show resolved Hide resolved
@iwysiu
Copy link
Contributor Author

iwysiu commented Jun 11, 2021

LGTM. Just compared with the release/1.4 backport and saw one extra change (seems harmless). Should you mark GODRIVER-1923 with release version 1.3.8?

For full intellectual honesty, you should probably compare it with the one linked up top, since I actually made a minor change to the 1.4 backport and forgot to file a pull request for it. this contains both the 1.4 and 1.3 backport changes, though.
And since we don't offically support this branch anymore, I wasn't planning to release it


t.Run("element keys", func(t *testing.T) {
createDocFn := func() {
BuildDocument(nil, AppendStringElement(nil, "a\x00", "foo"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing out the minor change from the commit on master here. Looks like NewDocumentBuilder is not available until 1.5.0 with GODRIVER-613.

LGTM even more, and no need to mark as 1.3.8 since we won't release on 1.3.

Copy link
Contributor

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

LGTM. Just compared with the release/1.4 backport and saw one extra change (seems harmless). Should you mark GODRIVER-1923 with release version 1.3.8?

Good question, I will check if a release is required. If it is, we may need to do a 1.3.8 and 1.4.8 release.

@iwysiu iwysiu merged commit 98af5b4 into mongodb:release/1.3 Jun 14, 2021
kevinAlbs pushed a commit to kevinAlbs/mongo-go-driver that referenced this pull request Aug 11, 2021
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