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

Add CloudflareProtectedEmailAddress #6

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions lib/cloudflare_protected_email_address.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# frozen_string_literal: true

require 'cgi'

class CloudflareProtectedEmailAddress
def initialize(obfuscated)
@obfuscated = obfuscated
end

def human_readable
return obfuscated unless unescaped.include?('@')
unescaped
end

private

attr_accessor :obfuscated

def components
Copy link
Contributor

Choose a reason for hiding this comment

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

components is a bit too overloaded a term for me to be entirely comfortable with it here — I'm not sure it really conveys what's actually going on, and could be misleading. Is there a better name you can think of?

obfuscated.scan(/.{2}/).map(&:hex)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious here why you're using /.{2}/ rather than just ..

end

def obfuscated_characters
Copy link
Contributor

Choose a reason for hiding this comment

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

This is actually quite a misleading name too, as it returns integers, not characters

components.drop(1)
end

def key
components.first
end

def escaped
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this would be easier to follow if it actually mentioned percent encoding (or hex encoding) somewhere. Ideally that would be obvious from the name of the method, though if that's not sensible, then a comment might be called for.

obfuscated_characters.map { |char| (char ^ key).to_s(16).prepend('%') }.join
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that I've worked out what's actually going on here, I'm not entirely sure that .to_s(16) gets you the correct format. This approach will convert 15 to '%f' when I think it's strictly meant to be '%0F'.

sprintf("#%02X" % i) should get you the correct value, and means you wouldn't need the prepend either.

end
Copy link
Contributor

Choose a reason for hiding this comment

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

key and obfuscated_characters here are both taking responsibility for the same thing, but doing it in slightly different ways. This would be cleaner and more maintainable if those both used an underlying method that took care of the conversion e.g.

  def hex_pairs
    obfuscated.scan(/.{2}/).map(&:hex)
  end

  def obfuscated_characters
    hex_pairs.drop(1)
  end

  def key
    hex_pairs.first
  end

(not sure if hex_pairs is the right name here)


def unescaped
CGI.unescape(escaped)
end
end
24 changes: 24 additions & 0 deletions test/cloudflare_protected_email_address_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# frozen_string_literal: true

require_relative './test_helper'
require_relative '../lib/cloudflare_protected_email_address'

describe CloudflareProtectedEmailAddress do
it 'will make an obfuscated email address human readable' do
CloudflareProtectedEmailAddress.new('b3d2dfd7dcc5d6c1d2f3d7dac3c6c7d2d7dcc09dd4dcc59dc3ca')
.human_readable
.must_equal '[email protected]'
end

it 'returns the initialization email when it is not obfuscated' do
CloudflareProtectedEmailAddress.new('[email protected]')
.human_readable
.must_equal '[email protected]'
end

it 'returns the original string when the initialization string is not an obfuscated email' do
CloudflareProtectedEmailAddress.new('xyz789')
.human_readable
.must_equal 'xyz789'
end
end