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 Regexp.from_bson for data from SSL connections #44

Merged
merged 1 commit into from
Oct 8, 2015

Conversation

niels
Copy link
Contributor

@niels niels commented Oct 6, 2015

This PR unifies parsing of regular expressions across SSL and non-SSL connections thus fixing RUBY-1048. From the issue description:

When accessing mongodb through SSL, Regexp attributes can't be read from the database. https://github.com/mongodb/bson-ruby/blob/master/lib/bson/regexp.rb#L156 hangs indefinitely as the options are NULL_BYTE terminated.

Note that this issue only occurs when connecting through SSL. On an unencrypted connection, options are indeed "0" terminated and the bug thus doesn't manifest itself using non-encrypted connections.

The parsing of the options itself is also subject to differences between SSL and non-SSL connections.

Test case in Rails:

class TestRecord
  include Mongoid::Document
  field :regex, type: Regexp
end 

TestRecord.create(regex: //).reload # Will never return when connecting to MongoDB through SSL

@estolfo
Copy link
Contributor

estolfo commented Oct 6, 2015

Hi @Nielsomat
Thanks so much for this. I've commented in the jira ticket. It'd be great if you could provide a test for the scenario your code addresses and rebase against master. Thanks!

@estolfo
Copy link
Contributor

estolfo commented Oct 7, 2015

Hi @Nielsomat

Let me know if you're able to write a test for this and rebase against master so I can merge it. Thanks!

Emily

@durran
Copy link
Member

durran commented Oct 8, 2015

@niels
Copy link
Contributor Author

niels commented Oct 8, 2015

@estolfo, this commit simply brings the #readbyte handling within Regexp in line with existing code such as https://github.com/mongodb/bson-ruby/blob/master/lib/bson/array.rb#L97 or https://github.com/mongodb/bson-ruby/blob/master/lib/bson/hash.rb#L77. As it is not explicitly tested there, I didn't include tests of my own. I have adapted the commit message accordingly.

It sounds like @durran's branch is the way to go in the long-term. For the short-term, I would suggest including this fix ASAP. The symptom of the bug isn't "just" invalid or incomplete data but rather the entire process hanging.

Right now, I am also working on a PR against mongodb/mongo-ruby-driver to address the underlying issue of the raw TCP and SSL socket behaving differently. I will reference this PR here once completed.

niels added a commit to herimedia/mongo-ruby-driver that referenced this pull request Oct 8, 2015
by returning an Integer byte rather than a String and raising EOFError
if EOF was reached.

This unifies #readbyte across SSL and non-SSL connections.

[also see mongodb/bson-ruby#44]
@niels
Copy link
Contributor Author

niels commented Oct 8, 2015

The root cause for this is now fixed in mongodb/mongo-ruby-driver#700.

durran added a commit that referenced this pull request Oct 8, 2015
Fix Regexp.from_bson for data from SSL connections
@durran durran merged commit 31fbe10 into mongodb:master Oct 8, 2015
@durran
Copy link
Member

durran commented Oct 8, 2015

3.2.6 also has been released with this change. :)

@niels
Copy link
Contributor Author

niels commented Oct 8, 2015

😍

Excellent! Thanks, @durran!

@niels niels deleted the fix-regexp-for-ssl branch October 8, 2015 13:45
durran pushed a commit to mongodb/mongo-ruby-driver that referenced this pull request Oct 8, 2015
by returning an Integer byte rather than a String and raising EOFError
if EOF was reached.

This unifies #readbyte across SSL and non-SSL connections.

[also see mongodb/bson-ruby#44]
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