Recently we were struggling with some performance issues - importing data was taking way longer than it should.

Finding the problem

Ruby profiler to the rescue! This gem represents everything I love about the Ruby community. Its open source, its intuitive, and it simply just works so I can back to building applications.

Profiler setup

The simplest setup to test specific code, comes straight from their README:

require 'ruby-prof'

# Profile the code
result = RubyProf.profile do
  ...
  [code to profile]
  ...
end

# Print a graph profile to text
printer = RubyProf::GraphPrinter.new(result)
printer.print(STDOUT, {})

The results come back and String#equals is showing up highly ranked. Granted, import is doing a lot of string manipulation - parsing, encrypting, etc… - but I wouldn’t expect it to show up as a performance problem.

We start reviewing code - especially the parts around encryption since files with encrypted data are taken even longer than the other files, and we find…

The Cause

The gem we were using for encryption, encrypted_strings, is patching String to implement a new ==.

It’s doing what?

Yup. Changing the default behavior of == on a String. All strings - encrypted or not.

module EncryptedStrings
  module Extensions #:nodoc:
    # Adds support for in-place encryption/decryption of strings
    module String
      def self.included(base) #:nodoc:
        base.class_eval do
          attr_accessor :cipher
          
          alias_method :equals_without_encryption, :==
          alias_method :==, :equals_with_encryption
        end
      end

...

# Tests whether the other object is equal to this one.  Encrypted strings
# will be tested not only on their encrypted strings, but also by
# decrypting them and running tests against the decrypted value.
# 
# == Equality with strings
# 
#   password = 'shhhh'
#   password.encrypt!   # => "66c85d26dadde7e1db27e15a0776c921e27143bd"
#   password            # => "66c85d26dadde7e1db27e15a0776c921e27143bd"
#   password == "shhhh" # => true
# 
# == Equality with encrypted strings
# 
#   password = 'shhhh'
#   password.encrypt!             # => "66c85d26dadde7e1db27e15a0776c921e27143bd"
#   password                      # => "66c85d26dadde7e1db27e15a0776c921e27143bd"
#   password == 'shhhh'           # => true
#   
#   another_password = 'shhhh'
#   another_password.encrypt!     # => "66c85d26dadde7e1db27e15a0776c921e27143bd"
#   password == another_password  # => true
def equals_with_encryption(other)
  if !(is_equal = equals_without_encryption(other)) && String === other
    if encrypted?
      if other.encrypted?
        # We're both encrypted, so check if:
        # (1) The other string is the encrypted value of this string
        # (2) This string is the encrypted value of the other string
        # (3) The other string is the encrypted value of this string, decrypted
        # (4) This string is the encrypted value of the other string, decrypted
        is_string_equal?(self, other) || is_string_equal?(other, self) || self.can_decrypt? && is_string_equal?(self.decrypt, other) || other.can_decrypt? && is_string_equal?(other.decrypt, self)
      else
        # Only we're encrypted
        is_string_equal?(other, self)
      end
    else
      if other.encrypted?
        # Only the other string is encrypted
        is_string_equal?(self, other)
      else
        # Neither are encrypted and equality test didn't work before, so
        # they can't be equal
        false
      end
    end
  else
    # The other value wasn't a string, so we can't check encryption equality
    is_equal
  end
end

Now, let me admit that we should take some of the blame when we selected this gem:

  • Admittedly, it was written in a week
  • It hasn’t had a commit in 2 years
  • We looked over the code, but missed this patch

But this is just a bad idea… Taking an operation like == and changing it for every String in the application is a warning that needs to be boldly posted on the front label.

WARNING:

This gem may make your application slow to a crawl!

Please proceed at your own risk!

I love that Ruby lets you do things like this, but it doesn’t mean you should.

And if you’re looking for a rock solid encryption gem that screams, try Gibberish

  • Categories
comments powered by Disqus