Add integration tests for HTTP API

7 files changed, 315 insertions, 71 deletions

diff --git a/Makefile b/Makefile
index 60867aa..dc4c4ed 100644
--- a/Makefile
+++ b/Makefile
@@ -17,16 +17,19 @@
 SHELL = /bin/sh
 PREFIX ?= /usr/local
 
-.PHONY: man all clean install uninstall
+.PHONY: man all clean check install uninstall
 
-all: bin
+all: hybring
 
-bin: $(wildcard src/*.cr)
+hybring: $(wildcard src/*.cr)
 	crystal build -o hybring src/cli.cr
 
 clean:
 	rm hybring
 
+check: $(wildcard spec/*.cr)
+	crystal spec --order random spec/server.cr
+
 install: all
 	install -Dm 755 hybring ${DESTDIR}${PREFIX}/bin/hybring
 
diff --git a/spec/helper.cr b/spec/helper.cr
new file mode 100644
index 0000000..6fe13e5
--- /dev/null
+++ b/spec/helper.cr
@@ -0,0 +1,29 @@
+# Spec helper
+# Copyright (C) 2023  Nguyễn Gia Phong
+#
+# This file if part of hybring.
+#
+# Hybring is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Hybring is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with hybring.  If not, see <https://www.gnu.org/licenses/>.
+
+require "spec"
+::SPEC = true
+
+require "../src/cli"
+
+CONFIG = Configuration.new({"general" => {"db" => File.tempname,
+                                          "api" => "/"},
+                            "opennic" => {"local" => File.tempname,
+                                          "remote" => "http://example.null"},
+                            "icann" => {"local" => File.tempname,
+                                          "remote" => "http://example.net"}})
diff --git a/spec/server.cr b/spec/server.cr
new file mode 100644
index 0000000..69bb543
--- /dev/null
+++ b/spec/server.cr
@@ -0,0 +1,186 @@
+# HTTP server spec
+# Copyright (C) 2023  Nguyễn Gia Phong
+#
+# This file if part of hybring.
+#
+# Hybring is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Hybring is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with hybring.  If not, see <https://www.gnu.org/licenses/>.
+
+require "file_utils"
+require "http/client"
+
+require "./helper"
+require "../src/http"
+require "../src/sqlite"
+
+BASE_DATA = {"nick" => "example",
+             "opennic" => "http://example.indy",
+             "icann" => "https://example.org",
+             "host" => "example.net"}
+
+def expect_errors(url, overlay, expected)
+  data = BASE_DATA.merge overlay
+  response = HTTP::Client.post url, form: URI::Params.encode data
+  response.status_code.should eq 400
+  response.content_type.should eq "application/xhtml+xml"
+  xml = XML.parse response.body
+  errors = {} of String => String
+  xml.xpath_nodes("//xhtml:form/xhtml:label[@class='error'][.!='Error:']",
+                  {"xhtml" => "http://www.w3.org/1999/xhtml"}).each do |node|
+    errors[node["for"]] = node.content
+  end
+  errors.should eq expected
+end
+
+url = uninitialized String
+Spec.before_suite do
+  server = Server.new CONFIG
+  spawn do
+    server.listen do |address|
+      url = "http://#{address}"
+    end
+  end
+  until server.listening?
+    sleep 1.milliseconds
+  end
+  Spec.after_suite do
+    server.close
+    FileUtils.rm_r [CONFIG.db, CONFIG.opennic_local, CONFIG.icann_local]
+  end
+end
+
+describe Server do
+  it "only accepts POST requests" do
+    %w(GET PUT HEAD DELETE PATCH OPTIONS).each do |method|
+      response = HTTP::Client.exec method, url
+      response.status_code.should eq 405
+    end
+  end
+
+  pending "requires Content-Length header" do
+    # FIXME: HTTP::Client automatically sets the header based on body
+  end
+
+  it "rejects too long content" do
+    response = HTTP::Client.post url, body: "x" * (MAX_CONTENT_LENGTH + 1)
+    response.status_code.should eq 413
+  end
+
+  it "only accepts application/x-www-form-urlencoded Content-Type" do
+    response = HTTP::Client.post url
+    response.status_code.should eq 415
+    %w(multipart/form-data text/html text/plain).each do |content_type|
+      headers = HTTP::Headers{"Content-Type" => content_type}
+      response = HTTP::Client.post url, headers
+      response.status_code.should eq 415
+    end
+  end
+
+  it "only allows parameters nick, opennic, icann or host" do
+    data = BASE_DATA.merge Hash{"foo" => "bar"}
+    response = HTTP::Client.post url, form: URI::Params.encode data
+    response.status_code.should eq 400
+    response.content_type.should eq "text/plain"
+    response.body.should eq "400 Invalid Parameter\n"
+  end
+
+  it "requires parameters nick, opennic, icann and host" do
+    k, keys = BASE_DATA.size, BASE_DATA.keys
+    until k = 0
+      keys.combinations(k).each do |c|
+        data = BASE_DATA.reject c
+        response = HTTP::Client.post url, form: URI::Params.encode data
+        response.status_code.should eq 400
+        response.content_type.should eq "text/plain"
+        response.body.should eq "400 Missing Parameter\n"
+      end
+      k -= 1
+    end
+  end
+
+  it "rejects too long nick" do
+    msg = "Must be within #{MAX_NICK_LENGTH} characters"
+    expect_errors url, {"nick" => "x" * (MAX_NICK_LENGTH + 1)}, {"nick" => msg}
+  end
+
+  it "rejects nicks that are not lowercase alphanumeric" do
+    %w(camelCase pun/tu?a#tion).each do |nick|
+      expect_errors url, {"nick" => nick},
+                    {"nick" => "Must be ASCII lowercase alphanumeric"}
+    end
+  end
+
+  it "rejects relative URLs" do
+    %w(foo /bar ex.am/ple).each do |uri|
+      expect_errors url, {"opennic" => uri, "icann" => uri},
+                    {"opennic" => "Must be absolute URL",
+                     "icann" => "Must be absolute URL"}
+    end
+  end
+
+  it "only accepts HTTP/S" do
+    expect_errors url, {"opennic" => "gopher://example.indy"},
+                  {"opennic" => "Must be HTTP/S"}
+    expect_errors url, {"opennic" => "https://example.indy",
+                        "icann" => "http://example.org"},
+                  {"icann" => "Must be HTTPS"}
+  end
+
+  it "checks for OpenNIC domain" do
+    expect_errors url, {"opennic" => "http://example.org",
+                        "icann" => "https://example.indy"},
+                  {"opennic" => "Must be under OpenNIC domain",
+                   "icann" => "Must not be under OpenNIC domain"}
+  end
+
+  it "checks for uniqueness of nick and URLs" do
+    overlay = {"nick" => "self", "opennic" => CONFIG.opennic_remote,
+               "icann" => CONFIG.icann_remote}
+    base_errors = Hash(String, String).new "Must be unique"
+    k, keys = overlay.size, overlay.keys
+    until k = 0
+      keys.combinations(k).each do |c|
+        errors = base_errors.select c
+        data = BASE_DATA.merge overlay.select errors.keys
+        expect_errors url, data, errors
+        k -= 1
+      end
+    end
+  end
+
+  it "prevents nick from colliding with HTML headings' id" do
+    msg = "Reserved names: #{HTML_HEADINGS.join ", "}"
+    HTML_HEADINGS.each do |nick|
+      expect_errors url, {"nick" => nick}, {"nick" => msg}
+    end
+  end
+
+  it "inserts to database upon successful application" do
+    overlay = {"nick" => "chad", "opennic" => "http://chad.epic",
+               "icann" => "https://chad.example"}
+    data = BASE_DATA.merge overlay
+    response = HTTP::Client.post url, form: URI::Params.encode data
+    response.status_code.should eq 200
+    response.content_type.should eq "application/xhtml+xml"
+    db = Database.new CONFIG.db
+    db.exec "SELECT opennic, icann, official, left, right
+             FROM member
+             WHERE nick = %Q", overlay["nick"] do |row|
+      row[0].text.should eq overlay["opennic"]
+      row[1].text.should eq overlay["icann"]
+      row[2].int.should eq 0 # applicant
+      row[3].text.should eq "self"
+      row[4].text.should eq "self"
+    end
+  end
+end
diff --git a/src/cli.cr b/src/cli.cr
index 6f5412c..8cc79be 100644
--- a/src/cli.cr
+++ b/src/cli.cr
@@ -40,8 +40,7 @@ struct Configuration
   @icann_remote : String
   getter icann_remote
 
-  def initialize(path)
-    ini = INI.parse File.read path
+  def initialize(ini)
     @db = Path[ini["general"]["db"]]
     @api = ini["general"]["api"]
     @opennic_local = Path[ini["opennic"]["local"]]
@@ -51,68 +50,74 @@ struct Configuration
   end
 end
 
-subcmd = Subcommand::Usage
-usage = ""
-config_path, port = nil, 8910
+def die(error)
+  STDERR << error
+  exit 1
+end
+
+{% unless @top_level.constant("SPEC") %}
+  subcmd = Subcommand::Usage
+  usage = ""
+  config_path, port = nil, 0
 
-parser = OptionParser.new do |parser|
-  banner_prefix = "Usage: #{Path[PROGRAM_NAME].basename}"
-  parser.banner = "#{banner_prefix} [subcommand] [arguments]"
-  parser.on "serve", "start the HTTP server"  do
-    subcmd = Subcommand::Serve
-    parser.banner = "#{banner_prefix} serve --config=PATH [--port=N]"
-    usage = parser.to_s
-    parser.on "-c PATH", "--config=PATH",
-              "path to configuration file (required)" do |path|
-      config_path = path
+  parser = OptionParser.new do |parser|
+    banner_prefix = "Usage: #{Path[PROGRAM_NAME].basename}"
+    parser.banner = "#{banner_prefix} [subcommand] [arguments]"
+    parser.on "serve", "start the HTTP server"  do
+      subcmd = Subcommand::Serve
+      parser.banner = "#{banner_prefix} serve --config=PATH [--port=N]"
+      usage = parser.to_s
+      parser.on "-c PATH", "--config=PATH",
+                "path to configuration file (required)" do |path|
+        config_path = path
+      end
+      parser.on "-p N", "--port=N", "listening port" do |n|
+        port = n.to_i
+      end
     end
-    parser.on "-p N", "--port=N", "listening port (default to 8910)" do |n|
-      port = n.to_i
+    parser.on "-h", "--help", "show this help message and exit"  do
+      puts parser
+      exit
     end
-  end
-  parser.on "-h", "--help", "show this help message and exit"  do
-    puts parser
-    exit
-  end
 
-  parser.invalid_option do |flag|
-    STDERR.puts "Error: Invalid option: #{flag}"
-    subcmd = Subcommand::Usage
-  end
-  parser.missing_option do |flag|
-    STDERR.puts "Error: Missing argument for #{flag}"
-    subcmd = Subcommand::Usage
-  end
-  parser.unknown_args do |before_dash, after_dash|
-    next if before_dash.empty? && after_dash.empty?
-    STDERR << "Error: Unknown arguments:"
-    STDERR << " " << before_dash.join " "  if !before_dash.empty?
-    STDERR << " -- " << after_dash.join " "  if !after_dash.empty?
-    STDERR.puts
-    subcmd = Subcommand::Usage
+    parser.invalid_option do |flag|
+      STDERR.puts "Error: Invalid option: #{flag}"
+      subcmd = Subcommand::Usage
+    end
+    parser.missing_option do |flag|
+      STDERR.puts "Error: Missing argument for #{flag}"
+      subcmd = Subcommand::Usage
+    end
+    parser.unknown_args do |before_dash, after_dash|
+      next if before_dash.empty? && after_dash.empty?
+      STDERR << "Error: Unknown arguments:"
+      STDERR << " " << before_dash.join " "  if !before_dash.empty?
+      STDERR << " -- " << after_dash.join " "  if !after_dash.empty?
+      STDERR.puts
+      subcmd = Subcommand::Usage
+    end
   end
-end
 
-parser.parse
-cfg = case subcmd
-      when .serve?
-        unless config_path
-          STDERR << usage
-          exit 1
+  parser.parse
+  cfg = case subcmd
+        when .serve?
+          unless config_path
+            die usage
+          end
+          begin
+            Configuration.new INI.parse File.read config_path.not_nil!
+          rescue ex
+            die "Error: Failed to read config from #{config_path}: #{ex}\n"
+          end
         end
-        begin
-          Configuration.new config_path.not_nil!
-        rescue ex
-          STDERR.puts "Error: Failed to read config from #{config_path}: #{ex}"
-          exit 1
-        end
-      end
 
-case subcmd
-in .serve?
-  server = Server.new cfg.not_nil!
-  server.listen port
-in .usage?
-  STDERR.puts parser
-  exit 1
-end
+  case subcmd
+  in .serve?
+    server = Server.new cfg.not_nil!
+    server.listen port do |address|
+      puts "Listening on http://#{address}"
+    end
+  in .usage?
+    die parser
+  end
+{% end %}
diff --git a/src/http.cr b/src/http.cr
index 912a46c..248a746 100644
--- a/src/http.cr
+++ b/src/http.cr
@@ -50,6 +50,10 @@ class Server
       content_length = context.request.content_length
       next http_error context, 411 unless content_length
       next http_error context, 413 if content_length > MAX_CONTENT_LENGTH
+      content_type = context.request.headers["Content-Type"]?
+      unless content_type && content_type == "application/x-www-form-urlencoded"
+        next http_error context, 415
+      end
 
       errors = {} of String => String
       params = {} of String => String
@@ -60,7 +64,7 @@ class Server
         case key
         when "nick"
           if value.size > MAX_NICK_LENGTH
-            next errors["nick"] = "Must be within ${MAX_NICK_LENGTH} characters"
+            next errors["nick"] = "Must be within #{MAX_NICK_LENGTH} characters"
           end
           if /^[0-9a-z]+$/ !~ value
             next errors["nick"] = "Must be ASCII lowercase alphanumeric"
@@ -124,7 +128,7 @@ class Server
   getter opennic_page
   getter icann_page
 
-  def listen(port)
+  def listen(port = 0)
     @db.update_hook ->(arg : Void*, action : Database::UpdateAction,
                        db : LibC::Char*, table : LibC::Char*, rowid : Int64) {
       return unless db == "main"
@@ -136,7 +140,15 @@ class Server
       end
     }, self.as Void*
 
-    puts "Listening on http://#{@server.bind_tcp port}"
+    yield @server.bind_tcp port
     @server.listen
   end
+
+  def listening?
+    @server.listening?
+  end
+
+  def close
+    @server.close
+  end
 end
diff --git a/src/sqlite.cr b/src/sqlite.cr
index 9ae1c51..60f2e19 100644
--- a/src/sqlite.cr
+++ b/src/sqlite.cr
@@ -19,6 +19,7 @@
 @[Link("sqlite3")]
 lib SQLite
   OK = 0
+  BUSY = 5
   ROW = 100
   DONE = 101
 
@@ -36,7 +37,7 @@ lib SQLite
   fun free = sqlite3_free(format : Void*, ...)
 
   fun open = sqlite3_open(filename : LibC::Char*, db : Database*) : LibC::Int
-  fun close = sqlite3_close(db : Database) : LibC::Int
+  fun close = sqlite3_close_v2(db : Database) : LibC::Int
   fun update_hook = sqlite3_update_hook(db : Database,
                                         f : (Void*, UpdateAction, LibC::Char*,
                                              LibC::Char*, Int64 ->),
@@ -119,6 +120,12 @@ class Database
     end
   end
 
+  def initialize(path)
+    Dir.mkdir_p path.parent
+    Database.check SQLite.open path.to_s, out @ref
+    self.exec "PRAGMA foreign_keys = ON" do end
+  end
+
   def initialize(path, opennic, icann)
     Dir.mkdir_p path.parent
     Database.check SQLite.open path.to_s, out @ref
@@ -162,6 +169,8 @@ class Database
         yield stmt.row
       when SQLite::DONE
         break
+      when SQLite::BUSY
+        next # FIXME: rollback transaction
       else
         Database.check rc
       end
@@ -169,7 +178,7 @@ class Database
   end
 
   def transact
-    self.exec "BEGIN TRANSACTION" do end
+    self.exec "BEGIN" do end
     yield
     self.exec "COMMIT" do end
   end
diff --git a/src/xhtml.cr b/src/xhtml.cr
index d53eb04..8db5995 100644
--- a/src/xhtml.cr
+++ b/src/xhtml.cr
@@ -32,9 +32,9 @@ CSS = "
           display: grid;
           grid-template-columns: max-content 1fr 0;
       }
-      form label { margin-right: 1ch }
       form input { margin-bottom: 1ex }
-      .error {
+      form label { margin-right: 1ch }
+      form label.error {
         color: red;
         margin-top: -1ex;
         margin-bottom: 1ex;
@@ -90,8 +90,8 @@ class Page
                 required: "required"
     xml.element "br"
     if error
-      xml.element "label", class: "error" do xml.text "Error:" end
-      xml.element "label", class: "error" do xml.text error end
+      xml.element "label", for: name, class: "error" do xml.text "Error:" end
+      xml.element "label", for: name, class: "error" do xml.text error end
       xml.element "br"
     end
   end