Refactoring

Makefile

@@ -10,7 +10,7 @@ MANDIR     	:= $(DESTDIR)$(PREFIX)/share/man
 
 STAMP	:= configure-stamp
 ifeq ($(wildcard $(STAMP)),)
-_ := $(shell ./configure)
+	_ := $(shell ./configure)
 endif
 
 #
@@ -27,11 +27,11 @@ LDFLAGS		:= -lpthread -lm $(OSLDFLAGS)
 CYGWIN_REQS	:= cygwin1.dll cygrunsrv.exe
 
 ifeq ($(CC),gcc)
-GCC_VER := $(shell ${CC} -dumpfullversion | sed -e 's/\.\([0-9][0-9]\)/\1/g' -e 's/\.\([0-9]\)/0\1/g' -e 's/^[0-9]\{3,4\}$$/&00/')
-GCC_GTEQ_430 := $(shell expr ${GCC_VER} \>= 40300)
-GCC_GTEQ_450 := $(shell expr ${GCC_VER} \>= 40500)
-GCC_GTEQ_600 := $(shell expr ${GCC_VER} \>= 60000)
-GCC_GTEQ_700 := $(shell expr ${GCC_VER} \>= 70000)
+	GCC_VER := $(shell ${CC} -dumpfullversion | sed -e 's/\.\([0-9][0-9]\)/\1/g' -e 's/\.\([0-9]\)/0\1/g' -e 's/^[0-9]\{3,4\}$$/&00/')
+	GCC_GTEQ_430 := $(shell expr ${GCC_VER} \>= 40300)
+	GCC_GTEQ_450 := $(shell expr ${GCC_VER} \>= 40500)
+	GCC_GTEQ_600 := $(shell expr ${GCC_VER} \>= 60000)
+	GCC_GTEQ_700 := $(shell expr ${GCC_VER} \>= 70000)
 endif
 
 CFLAGS	+= -std=c99 -D__BSD_VISIBLE -D_ALL_SOURCE -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=200112 -D_ISOC99_SOURCE -D_REENTRANT -D_BSD_SOURCE -D_DEFAULT_SOURCE -D_DARWIN_C_SOURCE -DVERSION=\"'$(VER)'\"
@@ -41,24 +41,24 @@ CFLAGS	+= -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
 #CFLAGS  += -fsanitize=undefined -fsanitize-undefined-trap-on-error
 
 ifeq ($(CC),gcc)
-ifeq "$(GCC_GTEQ_430)" "1"
-	CFLAGS += -Wlogical-op
-endif
-ifeq "$(GCC_GTEQ_450)" "1"
-	CFLAGS += -Wjump-misses-init
-endif
-ifeq "$(GCC_GTEQ_600)" "1"
-	CFLAGS += -Wduplicated-cond
-	CFLAGS += -Wnull-dereference
-	CFLAGS += -Werror=uninitialized
-	CFLAGS += -Wformat=2
-	CFLAGS += -Wformat-overflow=2
-	CFLAGS += -Wformat-truncation=2
-	CFLAGS += -Wformat-security
-endif
-ifeq "$(GCC_GTEQ_700)" "1"
-	CFLAGS += -Wduplicated-branches
-endif
+	ifeq "$(GCC_GTEQ_430)" "1"
+		CFLAGS += -Wlogical-op
+	endif
+	ifeq "$(GCC_GTEQ_450)" "1"
+		CFLAGS += -Wjump-misses-init
+	endif
+	ifeq "$(GCC_GTEQ_600)" "1"
+		CFLAGS += -Wduplicated-cond
+		CFLAGS += -Wnull-dereference
+		CFLAGS += -Werror=uninitialized
+		CFLAGS += -Wformat=2
+		CFLAGS += -Wformat-overflow=2
+		CFLAGS += -Wformat-truncation=2
+		CFLAGS += -Wformat-security
+	endif
+	ifeq "$(GCC_GTEQ_700)" "1"
+		CFLAGS += -Wduplicated-branches
+	endif
 endif
 
 #CFLAGS	+= -fstack-protector-strong
@@ -79,20 +79,23 @@ OBJS=main.o utils.o ntlm.o xcrypt.o config.o socket.o acl.o auth.o http.o forwar
 CONFIG_GSS=$(shell grep -c "config_gss 1" config/config.h)
 ifeq ($(CONFIG_GSS),1)
 	OBJS+=kerberos.o
-ifeq ($(OS),Darwin)
-	LDFLAGS+=-framework GSS
-else
-	LDFLAGS+=-lgssapi_krb5
-endif
+	ifeq ($(OS),Darwin)
+		LDFLAGS+=-framework GSS
+	else
+		LDFLAGS+=-lgssapi_krb5
+	endif
 endif
 
 ifneq ($(findstring CYGWIN,$(OS)),)
 	OBJS+=sspi.o win/resources.o
 endif
 
-ENABLE_STATIC=$(shell grep -c ENABLE_STATIC config/config.h)
+# Static linking is not available on macOS
+ENABLE_STATIC=$(shell grep -c ENABLE_STATIC $(STAMP))
 ifeq ($(ENABLE_STATIC),1)
-        LDFLAGS+=-static
+	ifneq ($(OS),Darwin)
+		LDFLAGS+=-static
+	endif
 endif
 
 CFLAGS_DUKTAPE := -Wno-bad-function-cast -Wno-null-dereference -Wno-format-nonliteral -Wno-unused-but-set-variable
@@ -106,15 +109,15 @@ $(NAME): $(OBJS)
 	@echo "Linking $@"
 	@$(CC) $(CFLAGS) -o $@ $(OBJS) $(LDFLAGS)
 
-main.o: main.c
+main.o: main.c *.h config/config.h
 	@echo "Compiling $<"
 	@if [ -z "$(SYSCONFDIR)" ]; then \
 		$(CC) $(CFLAGS) -c main.c -o $@; \
 	else \
 		$(CC) $(CFLAGS) -DSYSCONFDIR=\"$(SYSCONFDIR)\" -c main.c -o $@; \
 	fi
 
-%.o: %.c
+%.o: %.c *.h config/config.h
 	@echo "Compiling $<"
 	@$(CC) $(CFLAGS) -c -o $@ $<
 

VERSION

@@ -1 +1 @@
-0.94beta1
+0.94.0

auth.c

@@ -23,7 +23,6 @@
 #include <stdio.h>
 #include <string.h>
 
-#include "utils.h"
 #include "auth.h"
 
 struct auth_s *new_auth(void) {
@@ -33,24 +32,28 @@ struct auth_s *new_auth(void) {
 	if (tmp == NULL)
 		return NULL;
 
-	memset(tmp->user, 0, MINIBUF_SIZE);
-	memset(tmp->domain, 0, MINIBUF_SIZE);
-	memset(tmp->workstation, 0, MINIBUF_SIZE);
-	memset(tmp->passntlm2, 0, MINIBUF_SIZE);
-	memset(tmp->passnt, 0, MINIBUF_SIZE);
-	memset(tmp->passlm, 0, MINIBUF_SIZE);
+	return init_auth(tmp);
+}
+
+struct auth_s *init_auth(struct auth_s *creds) {
+	memset(creds->user, 0, MINIBUF_SIZE);
+	memset(creds->domain, 0, MINIBUF_SIZE);
+	memset(creds->workstation, 0, MINIBUF_SIZE);
+	memset(creds->passntlm2, 0, MINIBUF_SIZE);
+	memset(creds->passnt, 0, MINIBUF_SIZE);
+	memset(creds->passlm, 0, MINIBUF_SIZE);
 #ifdef __CYGWIN__
-	memset(&tmp->sspi, 0, sizeof(struct sspi_handle));
+	memset(&creds->sspi, 0, sizeof(struct sspi_handle));
 #endif
-	tmp->hashntlm2 = 1;
-	tmp->hashnt = 0;
-	tmp->hashlm = 0;
-	tmp->flags = 0;
+	creds->hashntlm2 = 1;
+	creds->hashnt = 0;
+	creds->hashlm = 0;
+	creds->flags = 0;
 #if config_gss == 1
-	tmp->haskrb = 0;
+	creds->haskrb = 0;
 #endif
 
-	return tmp;
+	return creds;
 }
 
 struct auth_s *copy_auth(struct auth_s *dst, const struct auth_s *src, int fullcopy) {

auth.h

@@ -68,6 +68,7 @@ struct auth_s {
  * new_auth() is also just a convenience malloc/memset() wrapper
  */
 extern struct auth_s *new_auth(void);
+extern struct auth_s *init_auth(struct auth_s *creds);
 extern struct auth_s *copy_auth(struct auth_s *dst, const struct auth_s *src, int fullcopy);
 extern struct auth_s *dup_auth(const struct auth_s *creds, int fullcopy);
 extern void dump_auth(const struct auth_s *creds);

config.c

@@ -24,10 +24,8 @@
 #include <string.h>
 #include <stdlib.h>
 
-#include "globals.h"
 #include "config.h"
-#include "utils.h"
-
+#include "globals.h"
 
 config_t config_open(const char *fname) {
 	config_t rc;
@@ -59,7 +57,7 @@ config_t config_open(const char *fname) {
 		/*
 		 * Find first non-empty character
 		 */
-		for (i = j = 0; j < len && isspace((u_char)buf[j]); ++j);
+		for (j = 0; j < len && isspace((u_char)buf[j]); ++j);
 
 		/*
 		 * Comment?
@@ -98,7 +96,7 @@ config_t config_open(const char *fname) {
 		/*
 		 * Find next non-empty character
 		 */
-		for (i = j; j < len && isspace((u_char)buf[j]); ++j);
+		for (; j < len && isspace((u_char)buf[j]); ++j);
 		if (j >= len || buf[j] == '#' || buf[j] == ';') {
 			free(key);
 			continue;

configure

@@ -63,28 +63,24 @@ for i in $TESTS; do
 	else
 		RET=$(./config/"$i")
 		rc=$?
-		[ -z "$RET" ] && if [ $rc -eq 0 ]; then RET="no"; else RET=yes; fi
+		[ -z "$RET" ] && if [ $rc -eq 0 ]; then RET="no"; else RET="yes"; fi
 	fi
 
 	echo $rc >> $CONFIG
 	echo "$RET"
 done
+echo "" >> $CONFIG
+echo "#endif // CONFIGURE_CONFIG_H" >> $CONFIG
 
 while [ "$1" ]
 do
 	case $1 in
 		--enable-static)
-			printf "#define ENABLE_STATIC" >> $CONFIG
-			echo "" >> $CONFIG
+			echo "ENABLE_STATIC" >> $STAMP
 			;;
 		*)
 			echo "Unknown flag $1"
-			rm -f $CONFIG
 			;;
 	esac
 	shift
 done
-if [ -f $CONFIG ]; then
-	echo "" >> $CONFIG
-	echo "#endif // CONFIGURE_CONFIG_H" >> $CONFIG
-fi

direct.c

@@ -31,13 +31,12 @@
 #include <netdb.h>
 #include <sys/socket.h>
 
-#include "utils.h"
+#include "direct.h"
 #include "globals.h"
 #include "auth.h"
 #include "http.h"
 #include "socket.h"
 #include "ntlm.h"
-#include "direct.h"
 #include "pages.h"
 
 int host_connect(const char *hostname, int port) {
@@ -258,8 +257,9 @@ rr_data_t direct_request(void *cdata, rr_data_const_t request) {
 		rsocket[1] = wsocket[0] = &sd;
 
 		conn_alive = 0;
+		loop = 0; // 0 = request from client; 1 = response from server
 
-		for (loop = 0; loop < 2; ++loop) {
+		while (loop < 2) {
 			if (data[loop]->empty) {				// Isn't this the first loop with request supplied by caller?
 				if (debug) {
 					printf("\n******* Round %d C: %d, S: %d *******\n", loop+1, cd, sd);
@@ -501,6 +501,8 @@ rr_data_t direct_request(void *cdata, rr_data_const_t request) {
 					goto bailout;
 				}
 			}
+
+			++loop;
 		}
 
 		free_rr_data(&data[0]);

forward.c

@@ -29,13 +29,11 @@
 #include <strings.h>
 #include <assert.h>
 
+#include "forward.h"
 #include "direct.h"
-#include "utils.h"
 #include "globals.h"
-#include "auth.h"
 #include "http.h"
 #include "socket.h"
-#include "forward.h"
 #include "scanner.h"
 #include "pages.h"
 #include "proxy.h"
@@ -100,7 +98,7 @@ rr_data_t forward_request(void *thread_data, rr_data_t request) {
 
 beginning:
 	sd = 0;
-	was_cached = noauth = authok = conn_alive = proxy_alive = 0;
+	was_cached = noauth = authok = proxy_alive = 0;
 
 	rsocket[0] = wsocket[1] = &cd;
 	rsocket[1] = wsocket[0] = &sd;
@@ -165,13 +163,14 @@ rr_data_t forward_request(void *thread_data, rr_data_t request) {
 		 *   - read proxy response
 		 *   - forward it to the client with HTTP body, if present
 		 *
-		 * There two goto's:
-		 *   - beginning: jump here to retry request (when cached connection timed out
+		 * There is one goto to "beginning":
+		 *   - jump here to retry request (when cached connection timed out
 		 *     or we thought proxy was notauth, but got 407)
-		 *   - shortcut: jump here from 1st iter. of inner loop, when we detect
-		 *     that auth isn't required by proxy. We do loop++, make the jump and
-		 *     the reply to our auth attempt (containing valid response) is sent to
-		 *     client directly without us making a request a second time.
+		 *
+		 * During 1st iter. of inner loop (loop == 0), when we detect
+		 * that auth isn't required by proxy, we set loop = 1 and
+		 * the reply to our auth attempt (containing valid response) is sent to
+		 * client directly without us making a request a second time.
 		 */
 		if (request) {
 			if (retry)
@@ -187,8 +186,9 @@ rr_data_t forward_request(void *thread_data, rr_data_t request) {
 		retry = 0;
 		proxy_alive = 0;
 		conn_alive = 0;
+		loop = 0; // 0 = request from client; 1 = response from server
 
-		for (loop = 0; loop < 2; ++loop) {
+		while (loop < 2) {
 			if (data[loop]->empty) {				// Isn't this the first loop with request supplied by caller?
 				if (debug) {
 					printf("\n******* Round %d C: %d, S: %d (authok=%d, noauth=%d) *******\n", loop+1, cd, sd, authok, noauth);
@@ -234,7 +234,6 @@ rr_data_t forward_request(void *thread_data, rr_data_t request) {
 				syslog(LOG_DEBUG, "%s %s %s", saddr, data[0]->method, data[0]->url);
 			}
 
-shortcut:
 			/*
 			 * Modify request headers.
 			 *
@@ -308,7 +307,7 @@ rr_data_t forward_request(void *thread_data, rr_data_t request) {
 				 * !!! that's why we reset data[1] below             !!!
 				 *
 				 * Reply to auth request wasn't 407? Then auth is not required,
-				 * let's jump into the next loop and forward it to client
+				 * let's set loop = 1 so that we forward reply to client
 				 * Also just forward if proxy doesn't reply with keep-alive,
 				 * because without it, NTLM auth wouldn't work anyway.
 				 *
@@ -321,14 +320,13 @@ rr_data_t forward_request(void *thread_data, rr_data_t request) {
 					if (data[1]->code < 400)
 						noauth = 1;
 					loop = 1;
-					goto shortcut;
+				} else {
+					/*
+					* If we're continuing normally, we have to free possible
+					* auth response from proxy_authenticate() in data[1]
+					*/
+					reset_rr_data(data[1]);
 				}
-
-				/*
-				 * If we're continuing normally, we have to free possible
-				 * auth response from proxy_authenticate() in data[1]
-				 */
-				reset_rr_data(data[1]);
 			}
 
 			/*
@@ -462,6 +460,8 @@ rr_data_t forward_request(void *thread_data, rr_data_t request) {
 					rc = (void *)-1;
 				}
 			}
+
+			++loop;
 		}
 
 		free_rr_data(&data[0]);

globals.h

@@ -32,8 +32,6 @@
 
 extern int debug;
 
-extern int request_logging_level;
-
 extern struct auth_s *g_creds;			/* global NTLM credentials */
 
 extern int ntlmbasic;				/* forward_request() */