From d378edca8215080fb0a86899f6dc52643bdb0852 Mon Sep 17 00:00:00 2001 From: Nguyễn Gia Phong Date: Tue, 7 Mar 2023 02:29:15 +0900 Subject: Add integration tests for HTTP API --- Makefile | 9 ++- spec/helper.cr | 29 +++++++++ spec/server.cr | 186 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/cli.cr | 123 ++++++++++++++++++++------------------ src/http.cr | 18 +++++- src/sqlite.cr | 13 +++- src/xhtml.cr | 8 +-- 7 files changed, 315 insertions(+), 71 deletions(-) create mode 100644 spec/helper.cr create mode 100644 spec/server.cr 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 . + +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 . + +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 -- cgit 1.4.1