Simplify API with recursive mutex

.gitignore

@@ -1,5 +1,6 @@
 *.o
 *.a
+*.so
 example_simple
 connection_afl
 findings

Makefile

@@ -1,38 +1,42 @@
 FIRE_CXX ?= clang++
-FIRE_CXXFLAGS ?= -O3 -std=gnu++2a -Wall -Werror
+FIRE_CXXFLAGS ?= -O3 -std=gnu++2a -Wall -Werror -Wextra -fPIE -fPIC -fstack-protector-strong -fsanitize=safe-stack -fsanitize=safe-stack
+FIRE_LDFLAGS ?= -fuse-ld=gold -flto -Wl,-z,relro -Wl,-z,now
 FIRE_LDLIBS ?= -lgflags -lglog -lpthread
 
-all: firecgi.a firecgi.o example_simple
+all: firecgi.a firecgi.o firecgi.so example_simple
 
 objects = server.o connection.o request.o parse.o
 
 firebuf/firebuf.o:
-	$(MAKE) --directory=firebuf
+	$(MAKE) --directory=firebuf firebuf.o
 
 firecgi.a: $(objects)
 	ar rcs $@ $^
 
 firecgi.o: $(objects) firebuf/firebuf.o
-	ld --relocatable --output=$@ $+
+	gold -z relro -z now -r --output=$@ $+
+
+firecgi.so: $(objects) firebuf/firebuf.o
+	$(FIRE_CXX) $(FIRE_CXXFLAGS) $(FIRE_LDFLAGS) -shared -o $@ $+ $(FIRE_LDFLIBS)
 
 example_simple: example_simple.o firecgi.o
-	$(FIRE_CXX) $(FIRE_CXXFLAGS) -o $@ $+ $(FIRE_LDLIBS)
+	$(FIRE_CXX) $(FIRE_CXXFLAGS) $(FIRE_LDFLAGS) -pie -o $@ $+ $(FIRE_LDLIBS)
 
 %.o: %.cc *.h Makefile
 	$(FIRE_CXX) $(FIRE_CXXFLAGS) -c -o $@ $<
 
 clean:
 	$(MAKE) --directory=firebuf clean
-	rm --force example_simple connection_afl *.o *.a
+	rm --force example_simple connection_afl *.so *.o *.a
 
 afl:
 	$(MAKE) clean
-	FIRE_CXX=afl-g++ $(MAKE) afl_int
+	FIRE_CXX=afl-g++ FIRE_CXXFLAGS="-O3 -std=gnu++2a -fPIC -fPIE" $(MAKE) afl_int
 
 afl_int: connection_afl
 
 connection_afl: connection_afl.o firecgi.o
-	$(FIRE_CXX) $(FIRE_CXXFLAGS) -o $@ $+ $(FIRE_LDLIBS)
+	$(FIRE_CXX) $(FIRE_CXXFLAGS) $(FIRE_LDFLAGS) -pie -o $@ $+ $(FIRE_LDLIBS)
 
 test: test_connection
 
@@ -43,4 +47,12 @@ test_connection: connection_afl
 
 asan:
 	$(MAKE) clean
-	FIRE_CXXFLAGS="-O1 -g -fsanitize=address -fno-omit-frame-pointer -std=gnu++2a -Wall -Werror" $(MAKE) all
+	FIRE_CXXFLAGS="-O1 -g -fsanitize=address -fno-omit-frame-pointer -std=gnu++2a -fPIE -fPIC" $(MAKE) all
+
+tsan:
+	$(MAKE) clean
+	FIRE_CXXFLAGS="-O1 -g -fsanitize=thread -std=gnu++2a -fPIE -fPIC" $(MAKE) all
+
+ubsan:
+	$(MAKE) clean
+	FIRE_CXXFLAGS="-O1 -g -fsanitize=undefined -std=gnu++2a -fPIE -fPIC" $(MAKE) all

connection.cc

@@ -9,11 +9,11 @@
 
 namespace firecgi {
 
-Connection::Connection(int sock, const sockaddr_in6& client_addr, const std::function<void(Request*)>& callback, const std::unordered_set<std::string_view>& headers)
+Connection::Connection(int sock, const sockaddr_in6& client_addr, const std::function<void(Request*)>& callback, const std::unordered_set<std::string_view>& headers, int max_request_len)
 		: sock_(sock),
 		  callback_(callback),
 		  headers_(headers),
-		  buf_(sock, max_record_len),
+		  buf_(sock, max_request_len),
 		  request_(this) {
 	char client_addr_str[INET6_ADDRSTRLEN];
 	PCHECK(inet_ntop(AF_INET6, &client_addr.sin6_addr, client_addr_str, sizeof(client_addr_str)));
@@ -122,9 +122,12 @@ int Connection::Read() {
 					// Magic signal for completed request (mirrors the HTTP/1.1 protocol)
 					requests_++;
 					callback_(&request_);
+					buf_.Consume(); // discard data and invalidate pointers
 				} else {
-					std::string_view in(buf_.Read(header->ContentLength()), header->ContentLength());
-					request_.AddIn(in);
+					if (!request_.GetBody().empty()) {
+						LOG(ERROR) << "received multiple stdin records. have you set \"fastcgi_request_buffering on\"?";
+					}
+					request_.SetBody({buf_.Read(header->ContentLength()), header->ContentLength()});
 				}
 			}
 			break;
@@ -141,7 +144,6 @@ int Connection::Read() {
 		buf_.Commit(); // we've acted on the bytes read so far
 	}
 
-	buf_.Consume();
 	return -1;
 }
 

connection.h

@@ -13,7 +13,7 @@ namespace firecgi {
 
 class Connection {
   public:
-	Connection(int sock, const sockaddr_in6& client_addr, const std::function<void(Request*)>& callback, const std::unordered_set<std::string_view>& headers);
+	Connection(int sock, const sockaddr_in6& client_addr, const std::function<void(Request*)>& callback, const std::unordered_set<std::string_view>& headers, int max_request_len);
 	~Connection();
 
 	[[nodiscard]] int Read();

connection_afl.cc

@@ -7,7 +7,7 @@ int main(int argc, char* argv[]) {
 	gflags::ParseCommandLineFlags(&argc, &argv, true);
 
 	{
-		firecgi::Connection conn(STDIN_FILENO, {}, [](std::unique_ptr<firecgi::Request> req) { req->End(); }, {});
+		firecgi::Connection conn(STDIN_FILENO, {}, [](firecgi::Request* req) { req->End(); }, {}, 16*1024);
 		static_cast<void>(conn.Read());
 	}
 

parse.h

@@ -44,8 +44,4 @@ struct ParamHeader {
 	uint8_t value_length;
 };
 
-constexpr auto max_content_len = 65535;
-constexpr auto max_padding_len = 255;
-constexpr auto max_record_len = sizeof(Header) + max_content_len + max_padding_len;
-
 } // namespace firecgi

request.cc

@@ -18,12 +18,12 @@ template<class T> void AppendVec(const T& obj, std::vector<iovec>* vec) {
 
 Request::Request(Connection* conn)
 		: conn_(conn),
-		  out_buf_(max_record_len) {}
+		  out_buf_(64*1024) {}
 
 void Request::NewRequest(uint16_t request_id) {
 	request_id_ = request_id;
 	params_.clear();
-	in_.clear();
+	body_ = {};
 	out_buf_.Reset();
 	body_written_ = false;
 }
@@ -36,20 +36,26 @@ void Request::AddParam(const std::string_view& key, const std::string_view& valu
 	params_.try_emplace(std::string(key), std::string(value));
 }
 
-void Request::AddIn(const std::string_view& in) {
-	in_.append(in);
+void Request::SetBody(const std::string_view& body) {
+	body_ = body;
 }
 
-const std::string& Request::GetParam(const std::string& key) {
+const std::string_view& Request::GetParam(const std::string& key) {
 	auto iter = params_.find(key);
 	if (iter == params_.end()) {
-		static const std::string none;
+		static const std::string_view none;
 		return none;
 	}
 	return iter->second;
 }
 
+const std::string_view& Request::GetBody() {
+	return body_;
+}
+
 void Request::WriteHeader(const std::string_view& name, const std::string_view& value) {
+	std::lock_guard<std::recursive_mutex> l(output_mu_);
+
 	CHECK(!body_written_);
 	CHECK(out_buf_.Write(name));
 	CHECK(out_buf_.Write(": "));
@@ -58,6 +64,7 @@ void Request::WriteHeader(const std::string_view& name, const std::string_view&
 }
 
 void Request::WriteBody(const std::string_view& body) {
+	std::lock_guard<std::recursive_mutex> l(output_mu_);
 	if (!body_written_) {
 		CHECK(out_buf_.Write("\n"));
 		body_written_ = true;
@@ -67,6 +74,8 @@ void Request::WriteBody(const std::string_view& body) {
 }
 
 bool Request::Flush() {
+	std::lock_guard<std::recursive_mutex> l(output_mu_);
+
 	std::vector<iovec> vecs;
 
 	auto header = OutputHeader();
@@ -78,11 +87,13 @@ bool Request::Flush() {
 		return false;
 	}
 	out_buf_.Commit();
+	out_buf_.Consume();
 	return true;
 }
 
 bool Request::End() {
-	// Fully empty response not allowed
+	std::lock_guard<std::recursive_mutex> l(output_mu_);
+
 	WriteBody("");
 
 	std::vector<iovec> vecs;

request.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <mutex>
 #include <unordered_map>
 
 #include "firebuf/buffer.h"
@@ -19,15 +20,19 @@ class Request {
 	uint16_t RequestId();
 
 	void AddParam(const std::string_view& key, const std::string_view& value);
-	void AddIn(const std::string_view& in);
+	void SetBody(const std::string_view& in);
 
-	const std::string& GetParam(const std::string& key);
+	const std::string_view& GetParam(const std::string& key);
+	const std::string_view& GetBody();
 
 	void WriteHeader(const std::string_view& name, const std::string_view& value);
 	void WriteBody(const std::string_view& body);
 	[[nodiscard]] bool Flush();
 	bool End();
 
+	template<typename...Args>
+	void WriteBody(const std::string_view& first, Args... more);
+
   private:
 	Header OutputHeader();
 	iovec OutputVec();
@@ -35,11 +40,19 @@ class Request {
 	Connection *conn_;
 	uint16_t request_id_ = 0;
 
-	std::unordered_map<std::string, std::string> params_;
-	std::string in_;
+	std::unordered_map<std::string_view, std::string_view> params_;
+	std::string_view body_;
 
 	firebuf::Buffer out_buf_;
 	bool body_written_;
+	std::recursive_mutex output_mu_;
 };
 
+template<typename...Args>
+void Request::WriteBody(const std::string_view& first, Args... more) {
+	std::lock_guard<std::recursive_mutex> l(output_mu_);
+	WriteBody(first);
+	WriteBody(more...);
+}
+
 } // namespace firecgi

server.cc

@@ -11,11 +11,12 @@
 
 namespace firecgi {
 
-Server::Server(int port, const std::function<void(Request*)>& callback, int threads, const std::unordered_set<std::string_view>& headers)
+Server::Server(int port, const std::function<void(Request*)>& callback, int threads, const std::unordered_set<std::string_view>& headers, int max_request_len)
 		: port_(port),
 		  callback_(callback),
 		  threads_(threads),
-		  headers_(headers) {
+		  headers_(headers),
+		  max_request_len_(max_request_len) {
 	LOG(INFO) << "listening on [::1]:" << port_;
 
 	signal(SIGPIPE, SIG_IGN);
@@ -81,7 +82,7 @@ void Server::NewConn(int listen_sock, int epoll_fd) {
 	PCHECK(setsockopt(client_sock, SOL_TCP, TCP_NODELAY, &flags, sizeof(flags)) == 0);
 
 	{
-		auto *conn = new Connection(client_sock, client_addr, callback_, headers_);
+		auto *conn = new Connection(client_sock, client_addr, callback_, headers_, max_request_len_);
 		struct epoll_event ev{
 			.events = EPOLLIN,
 			.data = {

server.h

@@ -10,7 +10,7 @@ namespace firecgi {
 
 class Server {
   public:
-	Server(int port, const std::function<void(Request*)>& callback, int threads=1, const std::unordered_set<std::string_view>& headers={});
+	Server(int port, const std::function<void(Request*)>& callback, int threads=1, const std::unordered_set<std::string_view>& headers={}, int max_request_len=(16*1024));
 	void Serve();
 
   private:
@@ -22,6 +22,7 @@ class Server {
 	const std::function<void(Request*)> callback_;
 	const int threads_;
 	const std::unordered_set<std::string_view> headers_;
+	const int max_request_len_;
 };
 
 } // firecgi