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

Integrate benburkert's interesting fixes + some test fixes #3

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ source "http://rubygems.org"

# Specify your gem's dependencies in gpgme.gemspec
gemspec

gem 'rake'
Copy link
Owner

Choose a reason for hiding this comment

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

How come this is needed? Never had any problems without it.

Copy link
Author

Choose a reason for hiding this comment

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

bundler complained, that rake is missing. This fixed the issue. To be honest I didn't investigate further than adding the dependency. I'm not sure if it hurts some day, some time. If you prefer, you can skip that commit.

13 changes: 7 additions & 6 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,18 @@ require 'yard'

desc "Re-compile the extensions"
task :compile do
FileUtils.rm_f('gpgme_n.bundle')
FileUtils.rm_f('gpgme_n.o')
FileUtils.rm_f('Makefile')
FileUtils.rm_rf('tmp') if File.directory?('tmp')
mkdir 'tmp'

system "ruby extconf.rb"
system "make"
Dir.chdir('tmp') do
system "ruby #{File.dirname(__FILE__)}/ext/gpgme/extconf.rb"
system "make"
end
end

task :default => [:test]

Rake::TestTask.new(:test) do |t|
Rake::TestTask.new(:test => :compile) do |t|
t.libs << 'test'
t.pattern = "test/**/*_test.rb"
t.verbose = true
Expand Down
22 changes: 21 additions & 1 deletion lib/gpgme/key.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class << self
#
# @example
# GPGME::Key.find(:public, "mrsimo@example.com", :sign)
# # => return the public keys that match mrsimo@exampl.com and are
# # => return the public keys that match mrsimo@example.com and are
# # capable of signing
def find(secret, keys_or_names = nil, purposes = [])
secret = (secret == :secret)
Expand All @@ -63,6 +63,26 @@ def find(secret, keys_or_names = nil, purposes = [])
keys
end

# Works similar as {.find}, however it restricts the way the keys are looked up.
# GPG has the issue that finding a key for bar@example.com, also returns a key
# for foobar@example.com.
# This can be restricted by adding <> around the address: <bar@example.com>.
# Hence {.find_exact} simply wraps <> around each email you passed to the method and delegates
# the rest to {.find}
#
# @example
# GPGME::Key.find_exact(:public, "bar@example.com")
# # => return the public key of bar@example.com, but not for
# # foobar@example.com
def find_exact(secret, keys_or_names = nil, purposes = [])
keys_or_names = [""] if keys_or_names.nil? || (keys_or_names.is_a?(Array) && keys_or_names.empty?)
find(
secret,
[keys_or_names].flatten.collect{|k| if k =~ /.*@.*/ && !(k =~ /<.*@.*>/) then "<#{k}>" else k end },
Copy link
Owner

Choose a reason for hiding this comment

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

I´m pretty concerned with this line. I need a lot of brain power to get to understand it, the 1 letter variable with two regexp´s and a ternary make it pretty hard.

Also, I´m not sure if this kind of method has to be part of the API. after all it´s 'normal' gpg behavior, so I have the feeling it's the user who should be passing emails in <> to the normal find instead. I'd probably add a comment on the find method explaining it, though.

Copy link
Author

Choose a reason for hiding this comment

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

The issue is the following:

Given you have 2 keys: one for bar@example.com and one for foobar@example.com. If you do a gpg --list-keys bar@example.com you get both keys. If you do gpg --list-keys '<bar@example.com>' you get only one key. To do the lookup with angle brackets is actually the solution that gpg developers recommend. A lot of existing tools assume that looking up a key for an address just returns one key or post-process the found keys and sort out the ones that don't match the exact pattern or are expired. Querying the keys with '<>' around means for example that you have only to sort out expired keys, but not think about possible other keys with totally unrelated keys. Imagine a key for a@example.com or o@example.com, which will likely return multiple times different keys.

I implemented that find_exact method in a couple of projects over and over again. This is also one reason why I started with rugpg. But given your work on ruby-gpgme, this would be the only method that is actually left over and which is a common use case for a lot of projects. Hence I propose and would love to see a direct integration in ruby-gpgme.

I agree that I tend to write compact rather than readable code. If you prefer, I could propose a more "readable" but less compact version.

Copy link
Author

Choose a reason for hiding this comment

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

Some in details about that can be found here: http://old.nabble.com/key-lookup-strategies-td30325738.html

purposes
)
end

def get(fingerprint)
Ctx.new do |ctx|
ctx.get_key(fingerprint)
Expand Down
22 changes: 22 additions & 0 deletions test/key_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,28 @@
assert keys.empty?
end
end

describe :find_exact do
it "wraps an email address with angle brackets" do
GPGME::Key.expects(:find).with(:public,['<bar@example.com>'],[])
GPGME::Key.find_exact(:public,'bar@example.com')
end

it "wraps multiple email addresses with angle brackets" do
GPGME::Key.expects(:find).with(:public,['<bar@example.com>','<foo@example.com>'],[])
GPGME::Key.find_exact(:public,['bar@example.com','foo@example.com'])
end

it "does not touch other strings than email addresses" do
GPGME::Key.expects(:find).with(:public,['Bar Example'],[])
GPGME::Key.find_exact(:public,'Bar Example')
end

it "does the same in mixed mode" do
GPGME::Key.expects(:find).with(:public,['<bar@example.com>','Foo Example'],[])
GPGME::Key.find_exact(:public,['bar@example.com','Foo Example'])
end
end

describe :export do
# Testing the lazy way with expectations. I think tests in
Expand Down
9 changes: 9 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
# -*- encoding: utf-8 -*-

# include compiled gpgme_n.bundle
tmp_dir = File.join(File.dirname(__FILE__), '..', 'tmp')
$:.unshift(tmp_dir) if File.directory?(tmp_dir)

# this interfers otherwise with our tests
ENV.delete('GPG_AGENT_INFO')

require 'rubygems'
require 'bundler/setup'
require 'minitest/autorun'
Expand All @@ -10,6 +18,7 @@

require File.dirname(__FILE__) + "/support/resources"


def import_keys
KEYS.each do |key|
import_key(key)
Expand Down