From fed9483e0e64d4b617cd7ba90fd34908181733e8 Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Mon, 30 Sep 2013 21:10:29 -0300 Subject: [PATCH 01/11] Ensure file is loaded with correct encoding This avoids encoding issues caused by building this gem with newer versions of Ruby. --- geoip-c.gemspec | 2 ++ 1 file changed, 2 insertions(+) diff --git a/geoip-c.gemspec b/geoip-c.gemspec index f4aed73..0bd2790 100644 --- a/geoip-c.gemspec +++ b/geoip-c.gemspec @@ -1,3 +1,5 @@ +# -*- encoding: utf-8 -*- + Gem::Specification.new do |s| s.name = 'geoip-c' s.version = "0.9.0" From 11583c0773a64330e66918e1eddfb895f4ece702 Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Mon, 30 Sep 2013 21:13:08 -0300 Subject: [PATCH 02/11] Update ignores - Avoid bundler configuration directory shows as modified - exclude only top level doc and pkg directories --- .gitignore | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 5bf7660..0f3c09c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ -pkg/ -doc/ +/.bundle +/pkg +/doc mkmf.log Makefile conftest.dSYM/ From 48c96cc11b13113808c601a67983c53412f4a103 Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Mon, 30 Sep 2013 21:14:31 -0300 Subject: [PATCH 03/11] Relocate extension files into ext/geoip folder Move things out of root of the gem to avoid things like: require "extconf" Messing things up Once extension is compiled it will be moved by RubyGems to lib directory, so there is no need to indicate "." be added to the $LOAD_PATH --- extconf.rb => ext/geoip/extconf.rb | 0 geoip.c => ext/geoip/geoip.c | 0 geoip-c.gemspec | 3 +-- 3 files changed, 1 insertion(+), 2 deletions(-) rename extconf.rb => ext/geoip/extconf.rb (100%) rename geoip.c => ext/geoip/geoip.c (100%) diff --git a/extconf.rb b/ext/geoip/extconf.rb similarity index 100% rename from extconf.rb rename to ext/geoip/extconf.rb diff --git a/geoip.c b/ext/geoip/geoip.c similarity index 100% rename from geoip.c rename to ext/geoip/geoip.c diff --git a/geoip-c.gemspec b/geoip-c.gemspec index 0bd2790..14c41d5 100644 --- a/geoip-c.gemspec +++ b/geoip-c.gemspec @@ -14,8 +14,7 @@ Gem::Specification.new do |s| s.files = `git ls-files`.split("\n") s.test_files = ['test.rb'] - s.extensions = ['extconf.rb'] - s.require_path = '.' + s.extensions = ["ext/geoip/extconf.rb"] s.add_development_dependency 'minitest', '~>5.0' s.add_development_dependency 'rake', '~>10.0' From ad879ad8e013aee99ffca836ab0f4d1d9a07de4a Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Wed, 16 Jan 2013 16:33:36 -0300 Subject: [PATCH 04/11] Use RDoc task for documentation This resolves the deprecation warning caused by usage of old rake. --- Rakefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Rakefile b/Rakefile index a56d68e..b1bfe95 100644 --- a/Rakefile +++ b/Rakefile @@ -9,9 +9,9 @@ task :default => [:compile, :test] CLEAN.add "geoip.{o,bundle,so,obj,pdb,lib,def,exp}" CLOBBER.add ['Makefile', 'mkmf.log','doc'] -Rake::RDocTask.new do |rdoc| - rdoc.rdoc_files.add ['README.md', 'geoip.c'] +RDoc::Task.new do |rdoc| rdoc.main = "README.md" # page to start on + rdoc.rdoc_files.add ["README.md", "ext/geoip/geoip.c"] rdoc.rdoc_dir = 'doc/' # rdoc output folder end From 76e3c37a9d3247196e43b6df8783a0ad79dbd113 Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Mon, 30 Sep 2013 21:34:06 -0300 Subject: [PATCH 05/11] Compile extension using rake-compiler This opens the door for cleaner compilation and later add cross compilation support. --- .gitignore | 1 + Rakefile | 11 ++++++----- geoip-c.gemspec | 7 ++++--- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index 0f3c09c..16b9e5f 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ geoip.bundle geoip.so geoip.o Gemfile.lock +/tmp diff --git a/Rakefile b/Rakefile index b1bfe95..89f7d5e 100644 --- a/Rakefile +++ b/Rakefile @@ -3,11 +3,11 @@ require 'bundler/gem_tasks' require 'rake/clean' require 'rake/testtask' require 'rdoc/task' +require "rake/extensiontask" task :default => [:compile, :test] -CLEAN.add "geoip.{o,bundle,so,obj,pdb,lib,def,exp}" -CLOBBER.add ['Makefile', 'mkmf.log','doc'] +CLOBBER.add 'doc' RDoc::Task.new do |rdoc| rdoc.main = "README.md" # page to start on @@ -20,6 +20,7 @@ Rake::TestTask.new do |t| t.verbose = true end -desc 'compile the extension' -task(:compile => 'Makefile') { sh 'make' } -file('Makefile' => "geoip.c") { ruby 'extconf.rb' } +spec = Gem::Specification.load "geoip-c.gemspec" + +Rake::ExtensionTask.new("geoip", spec) do |ext| +end diff --git a/geoip-c.gemspec b/geoip-c.gemspec index 14c41d5..9d48611 100644 --- a/geoip-c.gemspec +++ b/geoip-c.gemspec @@ -16,7 +16,8 @@ Gem::Specification.new do |s| s.test_files = ['test.rb'] s.extensions = ["ext/geoip/extconf.rb"] - s.add_development_dependency 'minitest', '~>5.0' - s.add_development_dependency 'rake', '~>10.0' - s.add_development_dependency 'rdoc', '~>4.0' + s.add_development_dependency 'minitest', '~> 5.0' + s.add_development_dependency 'rake', '~> 10.0' + s.add_development_dependency 'rdoc', '~> 4.0' + s.add_development_dependency "rake-compiler", "~> 0.9.1" end From e396d3b6f4ee9ef5157875c7131e32231505b620 Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Mon, 30 Sep 2013 21:37:31 -0300 Subject: [PATCH 06/11] Remove extra slash from doc task --- Rakefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index 89f7d5e..d6415aa 100644 --- a/Rakefile +++ b/Rakefile @@ -12,7 +12,7 @@ CLOBBER.add 'doc' RDoc::Task.new do |rdoc| rdoc.main = "README.md" # page to start on rdoc.rdoc_files.add ["README.md", "ext/geoip/geoip.c"] - rdoc.rdoc_dir = 'doc/' # rdoc output folder + rdoc.rdoc_dir = 'doc' # rdoc output folder end Rake::TestTask.new do |t| From f63f940db1d2d78f9ee36df7d9b8a2987c1c2ec4 Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Mon, 30 Sep 2013 21:38:11 -0300 Subject: [PATCH 07/11] Set test encoding header and remove full path require Without it, Ruby 1.9.x can't read the test file. Rake::TestTask automatically adds 'lib' directory to $LOAD_PATH. --- test.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test.rb b/test.rb index 49cc946..3bbbc32 100644 --- a/test.rb +++ b/test.rb @@ -1,8 +1,9 @@ # encoding: utf-8 + require 'rubygems' gem 'minitest' require 'minitest/autorun' -require File.dirname(__FILE__) + '/geoip' +require 'geoip' CITY_DB = ENV.fetch("CITY", '/usr/local/GeoIP/share/GeoIP/GeoLiteCity.dat') ORG_DB = ENV.fetch("ORG", '/usr/local/GeoIP/share/GeoIP/GeoIPOrg.dat') From 8efa958aaa485acc43b7ec6f65a4a161feedaacf Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Thu, 17 Jan 2013 12:05:23 -0300 Subject: [PATCH 08/11] Download GeoLiteCity database and use it on tests Clobber the data files from 'data' when requested. Also ignore data directory from Git. --- Rakefile | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index d6415aa..8eede90 100644 --- a/Rakefile +++ b/Rakefile @@ -7,7 +7,7 @@ require "rake/extensiontask" task :default => [:compile, :test] -CLOBBER.add 'doc' +CLOBBER.add 'doc', 'data' RDoc::Task.new do |rdoc| rdoc.main = "README.md" # page to start on @@ -24,3 +24,19 @@ spec = Gem::Specification.load "geoip-c.gemspec" Rake::ExtensionTask.new("geoip", spec) do |ext| end + +directory "data" + +file "data/GeoLiteCity.dat" => ["data"] do |f| + url = "http://geolite.maxmind.com/download/geoip/database/GeoLiteCity.dat.gz" + + sh "curl #{url} -o data/GeoLiteCity.dat.gz" + sh "gunzip data/GeoLiteCity.dat.gz" + touch f.name +end + +task :database => ["data/GeoLiteCity.dat"] do + ENV['CITY'] = File.expand_path("data/GeoLiteCity.dat") +end + +task :test => [:compile, :database] From ce270a90cc11a2a7b82fb9e4ba37098d08a8a5c5 Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Thu, 17 Jan 2013 12:06:16 -0300 Subject: [PATCH 09/11] Default task is to run tests Since test already depends on compile --- Rakefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index 8eede90..a645713 100644 --- a/Rakefile +++ b/Rakefile @@ -5,7 +5,7 @@ require 'rake/testtask' require 'rdoc/task' require "rake/extensiontask" -task :default => [:compile, :test] +task :default => [:test] CLOBBER.add 'doc', 'data' From 2d1144c8c721507732bd584caa64a4dd70565e32 Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Thu, 17 Jan 2013 12:28:49 -0300 Subject: [PATCH 10/11] Relocate test file to default 'test' directory And rename it to match the module it is testing. --- Rakefile | 1 - geoip-c.gemspec | 2 +- test.rb => test/test_geoip.rb | 0 3 files changed, 1 insertion(+), 2 deletions(-) rename test.rb => test/test_geoip.rb (100%) diff --git a/Rakefile b/Rakefile index a645713..207152c 100644 --- a/Rakefile +++ b/Rakefile @@ -16,7 +16,6 @@ RDoc::Task.new do |rdoc| end Rake::TestTask.new do |t| - t.test_files = ['test.rb'] t.verbose = true end diff --git a/geoip-c.gemspec b/geoip-c.gemspec index 9d48611..85f1485 100644 --- a/geoip-c.gemspec +++ b/geoip-c.gemspec @@ -13,7 +13,7 @@ Gem::Specification.new do |s| s.homepage = "http://github.com/mtodd/geoip" s.files = `git ls-files`.split("\n") - s.test_files = ['test.rb'] + s.test_files = ['test/test_geoip.rb'] s.extensions = ["ext/geoip/extconf.rb"] s.add_development_dependency 'minitest', '~> 5.0' diff --git a/test.rb b/test/test_geoip.rb similarity index 100% rename from test.rb rename to test/test_geoip.rb From 8843b4f1bcd5cdb9d4c9820a0564d8f338075c0f Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Mon, 30 Sep 2013 21:41:52 -0300 Subject: [PATCH 11/11] Avoid segfault caused by missing region_name For some country+region combos, region_name might not be found, which results in a NULL string and causes segfault when strlen() is performed on it. Move `region_name` variable definition to conform C89 and please some compilers. Add a test that verifies the fixed behavior. --- ext/geoip/geoip.c | 6 +++--- test/test_geoip.rb | 5 ++++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/ext/geoip/geoip.c b/ext/geoip/geoip.c index f218101..3ae8591 100644 --- a/ext/geoip/geoip.c +++ b/ext/geoip/geoip.c @@ -137,6 +137,7 @@ static VALUE generic_single_value_lookup_response(char *key, char *value) VALUE rb_city_record_to_hash(GeoIPRecord *record) { VALUE hash = rb_hash_new(); + char *region_name; if(record->country_code) rb_hash_sset(hash, "country_code", encode_to_utf8_and_return_rb_str(record->country_code)); @@ -147,10 +148,9 @@ VALUE rb_city_record_to_hash(GeoIPRecord *record) if(record->region) { rb_hash_sset(hash, "region", encode_to_utf8_and_return_rb_str(record->region)); - char *region_name = GeoIP_region_name_by_code(record->country_code, record->region); - if (region_name) { + region_name = GeoIP_region_name_by_code(record->country_code, record->region); + if (region_name) rb_hash_sset(hash, "region_name", encode_to_utf8_and_return_rb_str(region_name)); - } } if(record->city) rb_hash_sset(hash, "city", encode_to_utf8_and_return_rb_str(record->city)); diff --git a/test/test_geoip.rb b/test/test_geoip.rb index 3bbbc32..259a0c1 100644 --- a/test/test_geoip.rb +++ b/test/test_geoip.rb @@ -125,6 +125,10 @@ def test_empty_region_name_does_not_crash assert_look_up(db, '119.236.232.169', :region_name, nil) end + def test_hong_kong_segfault + db = GeoIP::City.new(@dbfile, :filesystem, true) + assert_look_up(db, "61.93.14.4", :country_name, "Hong Kong") + end end class GeoIPOrgTest < Minitest::Test @@ -171,5 +175,4 @@ def test_bad_db_file GeoIP::Organization.new('/supposed-to-fail') end end - end