fix lint

cgi.go

@@ -33,15 +33,15 @@ const (
 	sslProtocol
 )
 
-func allocServerVariable(cArr *[27]*C.char, env map[string]string, serverKey serverKey, envKey string, val string) {
+func allocServerVariable(cArr *[27]*C.char, env map[string]string, serverKey serverKey, envKey string, val string, pointers *pointerList) {
 	if val, ok := env[envKey]; ok {
-		cArr[serverKey] = C.CString(val)
+		cArr[serverKey] = pointers.ToCString(val)
 		delete(env, envKey)
 
 		return
 	}
 
-	cArr[serverKey] = C.CString(val)
+	cArr[serverKey] = pointers.ToCString(val)
 }
 
 // computeKnownVariables returns a set of CGI environment variables for the request.
@@ -53,6 +53,7 @@ func computeKnownVariables(request *http.Request) (cArr [27]*C.char) {
 	if !fcOK {
 		panic("not a FrankenPHP request")
 	}
+	pointers := getPointersForRequest(request)
 
 	// Separate remote IP and port; more lenient than net.SplitHostPort
 	var ip, port string
@@ -69,30 +70,30 @@ func computeKnownVariables(request *http.Request) (cArr [27]*C.char) {
 
 	ra, raOK := fc.env["REMOTE_ADDR"]
 	if raOK {
-		cArr[remoteAddr] = C.CString(ra)
+		cArr[remoteAddr] = pointers.ToCString(ra)
 		delete(fc.env, "REMOTE_ADDR")
 	} else {
-		cArr[remoteAddr] = C.CString(ip)
+		cArr[remoteAddr] = pointers.ToCString(ip)
 	}
 
 	if rh, ok := fc.env["REMOTE_HOST"]; ok {
-		cArr[remoteHost] = C.CString(rh) // For speed, remote host lookups disabled
+		cArr[remoteHost] = pointers.ToCString(rh) // For speed, remote host lookups disabled
 		delete(fc.env, "REMOTE_HOST")
 	} else {
 		if raOK {
-			cArr[remoteHost] = C.CString(ip)
+			cArr[remoteHost] = pointers.ToCString(ip)
 		} else {
 			cArr[remoteHost] = cArr[remoteAddr]
 		}
 	}
 
-	allocServerVariable(&cArr, fc.env, remotePort, "REMOTE_PORT", port)
-	allocServerVariable(&cArr, fc.env, documentRoot, "DOCUMENT_ROOT", fc.documentRoot)
-	allocServerVariable(&cArr, fc.env, pathInfo, "PATH_INFO", fc.pathInfo)
-	allocServerVariable(&cArr, fc.env, phpSelf, "PHP_SELF", request.URL.Path)
-	allocServerVariable(&cArr, fc.env, documentUri, "DOCUMENT_URI", fc.docURI)
-	allocServerVariable(&cArr, fc.env, scriptFilename, "SCRIPT_FILENAME", fc.scriptFilename)
-	allocServerVariable(&cArr, fc.env, scriptName, "SCRIPT_NAME", fc.scriptName)
+	allocServerVariable(&cArr, fc.env, remotePort, "REMOTE_PORT", port, pointers)
+	allocServerVariable(&cArr, fc.env, documentRoot, "DOCUMENT_ROOT", fc.documentRoot, pointers)
+	allocServerVariable(&cArr, fc.env, pathInfo, "PATH_INFO", fc.pathInfo, pointers)
+	allocServerVariable(&cArr, fc.env, phpSelf, "PHP_SELF", request.URL.Path, pointers)
+	allocServerVariable(&cArr, fc.env, documentUri, "DOCUMENT_URI", fc.docURI, pointers)
+	allocServerVariable(&cArr, fc.env, scriptFilename, "SCRIPT_FILENAME", fc.scriptFilename, pointers)
+	allocServerVariable(&cArr, fc.env, scriptName, "SCRIPT_NAME", fc.scriptName, pointers)
 
 	var rs string
 	if request.TLS == nil {
@@ -101,24 +102,24 @@ func computeKnownVariables(request *http.Request) (cArr [27]*C.char) {
 		rs = "https"
 
 		if h, ok := fc.env["HTTPS"]; ok {
-			cArr[https] = C.CString(h)
+			cArr[https] = pointers.ToCString(h)
 			delete(fc.env, "HTTPS")
 		} else {
-			cArr[https] = C.CString("on")
+			cArr[https] = pointers.ToCString("on")
 		}
 
 		// and pass the protocol details in a manner compatible with apache's mod_ssl
 		// (which is why these have a SSL_ prefix and not TLS_).
 		if p, ok := fc.env["SSL_PROTOCOL"]; ok {
-			cArr[sslProtocol] = C.CString(p)
+			cArr[sslProtocol] = pointers.ToCString(p)
 			delete(fc.env, "SSL_PROTOCOL")
 		} else {
 			if v, ok := tlsProtocolStrings[request.TLS.Version]; ok {
-				cArr[sslProtocol] = C.CString(v)
+				cArr[sslProtocol] = pointers.ToCString(v)
 			}
 		}
 	}
-	allocServerVariable(&cArr, fc.env, requestScheme, "REQUEST_SCHEME", rs)
+	allocServerVariable(&cArr, fc.env, requestScheme, "REQUEST_SCHEME", rs, pointers)
 
 	reqHost, reqPort, _ := net.SplitHostPort(request.Host)
 
@@ -140,9 +141,9 @@ func computeKnownVariables(request *http.Request) (cArr [27]*C.char) {
 		}
 	}
 
-	allocServerVariable(&cArr, fc.env, serverName, "SERVER_NAME", reqHost)
+	allocServerVariable(&cArr, fc.env, serverName, "SERVER_NAME", reqHost, pointers)
 	if reqPort != "" {
-		allocServerVariable(&cArr, fc.env, serverPort, "SERVER_PORT", reqPort)
+		allocServerVariable(&cArr, fc.env, serverPort, "SERVER_PORT", reqPort, pointers)
 	}
 
 	// Variables defined in CGI 1.1 spec
@@ -150,12 +151,12 @@ func computeKnownVariables(request *http.Request) (cArr [27]*C.char) {
 	// the parent environment from interfering.
 
 	// These values can not be override
-	cArr[contentLength] = C.CString(request.Header.Get("Content-Length"))
+	cArr[contentLength] = pointers.ToCString(request.Header.Get("Content-Length"))
 
-	allocServerVariable(&cArr, fc.env, gatewayInterface, "GATEWAY_INTERFACE", "CGI/1.1")
-	allocServerVariable(&cArr, fc.env, serverProtocol, "SERVER_PROTOCOL", request.Proto)
-	allocServerVariable(&cArr, fc.env, serverSoftware, "SERVER_SOFTWARE", "FrankenPHP")
-	allocServerVariable(&cArr, fc.env, httpHost, "HTTP_HOST", request.Host) // added here, since not always part of headers
+	allocServerVariable(&cArr, fc.env, gatewayInterface, "GATEWAY_INTERFACE", "CGI/1.1", pointers)
+	allocServerVariable(&cArr, fc.env, serverProtocol, "SERVER_PROTOCOL", request.Proto, pointers)
+	allocServerVariable(&cArr, fc.env, serverSoftware, "SERVER_SOFTWARE", "FrankenPHP", pointers)
+	allocServerVariable(&cArr, fc.env, httpHost, "HTTP_HOST", request.Host, pointers) // added here, since not always part of headers
 
 	return
 }

frankenphp.c

@@ -78,25 +78,12 @@ static uintptr_t frankenphp_clean_server_context() {
     return 0;
   }
 
-  free(SG(request_info).auth_password);
   SG(request_info).auth_password = NULL;
-
-  free(SG(request_info).auth_user);
   SG(request_info).auth_user = NULL;
-
-  free((char *)SG(request_info).request_method);
   SG(request_info).request_method = NULL;
-
-  free(SG(request_info).query_string);
   SG(request_info).query_string = NULL;
-
-  free((char *)SG(request_info).content_type);
   SG(request_info).content_type = NULL;
-
-  free(SG(request_info).path_translated);
   SG(request_info).path_translated = NULL;
-
-  free(SG(request_info).request_uri);
   SG(request_info).request_uri = NULL;
 
   return ctx->current_request;
@@ -352,7 +339,6 @@ static uintptr_t frankenphp_request_shutdown() {
 
   php_request_shutdown((void *)0);
 
-  free(ctx->cookie_data);
   ((frankenphp_server_context *)SG(server_context))->cookie_data = NULL;
   uintptr_t rh = frankenphp_clean_server_context();
 
@@ -508,11 +494,6 @@ static void frankenphp_register_known_variable(const char *key, char *value,
                                &new_val_len)) {
     php_register_variable_safe(key, value, new_val_len, track_vars_array);
   }
-
-  if (f) {
-    free(value);
-    value = NULL;
-  }
 }
 
 void frankenphp_register_bulk_variables(char *known_variables[27],
@@ -590,12 +571,7 @@ void frankenphp_register_bulk_variables(char *known_variables[27],
       php_register_variable_safe(dynamic_variables[i], dynamic_variables[i + 1],
                                  new_val_len, track_vars_array);
     }
-
-    free(dynamic_variables[i]);
-    free(dynamic_variables[i + 1]);
   }
-
-  free(dynamic_variables);
 }
 
 static void frankenphp_register_variables(zval *track_vars_array) {
@@ -729,8 +705,6 @@ int frankenphp_request_startup() {
 
 int frankenphp_execute_script(char *file_name) {
   if (frankenphp_request_startup() == FAILURE) {
-    free(file_name);
-
     return FAILURE;
   }
 
@@ -738,7 +712,6 @@ int frankenphp_execute_script(char *file_name) {
 
   zend_file_handle file_handle;
   zend_stream_init_filename(&file_handle, file_name);
-  free(file_name);
 
   file_handle.primary_script = 1;
 

frankenphp.go

@@ -54,9 +54,11 @@ import (
 
 type contextKeyStruct struct{}
 type handleKeyStruct struct{}
+type pointerKeyStruct struct{}
 
 var contextKey = contextKeyStruct{}
 var handleKey = handleKeyStruct{}
+var pointerKey = pointerKeyStruct{}
 
 var (
 	InvalidRequestError         = errors.New("not a FrankenPHP request")
@@ -195,6 +197,7 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques
 
 	c := context.WithValue(r.Context(), contextKey, fc)
 	c = context.WithValue(c, handleKey, Handles())
+	c = context.WithValue(c, pointerKey, Pointers())
 
 	return r.WithContext(c), nil
 }
@@ -362,23 +365,29 @@ func getLogger() *zap.Logger {
 	return logger
 }
 
+func getPointersForRequest(r *http.Request) *pointerList {
+	return r.Context().Value(pointerKey).(*pointerList)
+}
+
 func updateServerContext(request *http.Request, create bool, mrh C.uintptr_t) error {
 	fc, ok := FromContext(request.Context())
 	if !ok {
 		return InvalidRequestError
 	}
 
+	pointers := getPointersForRequest(request)
+
 	authUser, authPassword, ok := request.BasicAuth()
 	var cAuthUser, cAuthPassword *C.char
 	if ok && authPassword != "" {
-		cAuthPassword = C.CString(authPassword)
+		cAuthPassword = pointers.ToCString(authPassword)
 	}
 	if ok && authUser != "" {
-		cAuthUser = C.CString(authUser)
+		cAuthUser = pointers.ToCString(authUser)
 	}
 
-	cMethod := C.CString(request.Method)
-	cQueryString := C.CString(request.URL.RawQuery)
+	cMethod := pointers.ToCString(request.Method)
+	cQueryString := pointers.ToCString(request.URL.RawQuery)
 	contentLengthStr := request.Header.Get("Content-Length")
 	contentLength := 0
 	if contentLengthStr != "" {
@@ -392,7 +401,7 @@ func updateServerContext(request *http.Request, create bool, mrh C.uintptr_t) er
 	contentType := request.Header.Get("Content-Type")
 	var cContentType *C.char
 	if contentType != "" {
-		cContentType = C.CString(contentType)
+		cContentType = pointers.ToCString(contentType)
 	}
 
 	// compliance with the CGI specification requires that
@@ -400,10 +409,10 @@ func updateServerContext(request *http.Request, create bool, mrh C.uintptr_t) er
 	// Info: https://www.ietf.org/rfc/rfc3875 Page 14
 	var cPathTranslated *C.char
 	if fc.pathInfo != "" {
-		cPathTranslated = C.CString(sanitizedPathJoin(fc.documentRoot, fc.pathInfo)) // Info: http://www.oreilly.com/openbook/cgi/ch02_04.html
+		cPathTranslated = pointers.ToCString(sanitizedPathJoin(fc.documentRoot, fc.pathInfo)) // Info: http://www.oreilly.com/openbook/cgi/ch02_04.html
 	}
 
-	cRequestUri := C.CString(request.URL.RequestURI())
+	cRequestUri := pointers.ToCString(request.URL.RequestURI())
 
 	var rh cgo.Handle
 	if fc.responseWriter == nil {
@@ -498,17 +507,17 @@ func go_execute_script(rh unsafe.Pointer) {
 	if !ok {
 		panic(InvalidRequestError)
 	}
+	pointers := getPointersForRequest(request)
 	defer func() {
 		maybeCloseContext(fc)
-		request.Context().Value(handleKey).(*handleList).FreeAll()
+		finalizeRequest(request)
 	}()
 
 	if err := updateServerContext(request, true, 0); err != nil {
 		panic(err)
 	}
 
-	// scriptFilename is freed in frankenphp_execute_script()
-	fc.exitStatus = C.frankenphp_execute_script(C.CString(fc.scriptFilename))
+	fc.exitStatus = C.frankenphp_execute_script(pointers.ToCString(fc.scriptFilename))
 	if fc.exitStatus < 0 {
 		panic(ScriptExecutionError)
 	}
@@ -544,10 +553,10 @@ func go_ub_write(rh C.uintptr_t, cBuf *C.char, length C.int) (C.size_t, C.bool)
 func go_register_variables(rh C.uintptr_t, trackVarsArray *C.zval) {
 	r := cgo.Handle(rh).Value().(*http.Request)
 	fc := r.Context().Value(contextKey).(*FrankenPHPContext)
+	pointers := getPointersForRequest(r)
 
 	le := (len(fc.env) + len(r.Header)) * 2
-	dynamicVariablesArr := (**C.char)(C.malloc(C.size_t(le) * C.size_t(unsafe.Sizeof((*C.char)(nil)))))
-	dynamicVariables := unsafe.Slice(dynamicVariablesArr, le)
+	dynamicVariables := make([]*C.char, le)
 
 	var i int
 	// Add all HTTP headers to env variables
@@ -557,23 +566,28 @@ func go_register_variables(rh C.uintptr_t, trackVarsArray *C.zval) {
 			continue
 		}
 
-		dynamicVariables[i] = C.CString(k)
+		dynamicVariables[i] = pointers.ToCString(k)
 		i++
 
-		dynamicVariables[i] = C.CString(strings.Join(val, ", "))
+		dynamicVariables[i] = pointers.ToCString(strings.Join(val, ", "))
 		i++
 	}
 
 	for k, v := range fc.env {
-		dynamicVariables[i] = C.CString(k)
+		dynamicVariables[i] = pointers.ToCString(k)
 		i++
 
-		dynamicVariables[i] = C.CString(v)
+		dynamicVariables[i] = pointers.ToCString(v)
 		i++
 	}
 
+	var dynamicVariablesPtr **C.char = nil
+	if le > 0 {
+		dynamicVariablesPtr = &dynamicVariables[0]
+	}
+
 	knownVariables := computeKnownVariables(r)
-	C.frankenphp_register_bulk_variables(&knownVariables[0], dynamicVariablesArr, C.size_t(le), trackVarsArray)
+	C.frankenphp_register_bulk_variables(&knownVariables[0], dynamicVariablesPtr, C.size_t(le), trackVarsArray)
 
 	fc.env = nil
 }
@@ -664,6 +678,7 @@ func go_read_post(rh C.uintptr_t, cBuf *C.char, countBytes C.size_t) (readBytes
 //export go_read_cookies
 func go_read_cookies(rh C.uintptr_t) *C.char {
 	r := cgo.Handle(rh).Value().(*http.Request)
+	pointers := getPointersForRequest(r)
 
 	cookies := r.Cookies()
 	if len(cookies) == 0 {
@@ -676,7 +691,7 @@ func go_read_cookies(rh C.uintptr_t) *C.char {
 	}
 
 	// freed in frankenphp_request_shutdown()
-	return C.CString(strings.Join(cookieString, "; "))
+	return pointers.ToCString(strings.Join(cookieString, "; "))
 }
 
 //export go_log
@@ -732,3 +747,8 @@ func freeArgs(argv []*C.char) {
 		C.free(unsafe.Pointer(arg))
 	}
 }
+
+func finalizeRequest(r *http.Request) {
+	getPointersForRequest(r).FreeAll()
+	r.Context().Value(handleKey).(*handleList).FreeAll()
+}

smartpointer.go

@@ -34,9 +34,26 @@ func (p *pointerList) AddPointer(ptr unsafe.Pointer) {
 	p.Pointers = append(p.Pointers, ptr)
 }
 
+// AddString adds a string to the pointer list by converting it to a C-char pointer and calling the AddPointer method.
+// The string is converted to a C-char pointer using the C.CString function.
+// It is recommended to use this method when you need to add a string to the pointer list.
+func (p *pointerList) AddString(str *C.char) {
+	p.AddPointer(unsafe.Pointer(str))
+	//getLogger().Warn("Adding string", zap.Int("i", len(p.Pointers)), zap.String("str", C.GoString(str)), zap.Stack("trace"))
+}
+
+// ToCString takes a string and converts it to a C string using C.CString. Then it calls the AddString method of
+// pointerList to add the resulting C string as a pointer to the pointer
+func (p *pointerList) ToCString(string string) *C.char {
+	str := C.CString(string)
+	p.AddString(str)
+	return str
+}
+
 // FreeAll frees all C pointers
 func (p *pointerList) FreeAll() {
 	for _, ptr := range p.Pointers {
+		//getLogger().Warn("About to delete", zap.Int("i", i))
 		C.free(ptr)
 	}
 	p.Pointers = nil // To avoid dangling pointers

worker.go

@@ -170,7 +170,7 @@ func go_frankenphp_finish_request(mrh, rh C.uintptr_t, deleteHandle bool) {
 	fc := r.Context().Value(contextKey).(*FrankenPHPContext)
 
 	if deleteHandle {
-		r.Context().Value(handleKey).(*handleList).FreeAll()
+		finalizeRequest(r)
 		cgo.Handle(mrh).Value().(*http.Request).Context().Value(contextKey).(*FrankenPHPContext).currentWorkerRequest = 0
 	}