Skip to content

Commit 0c2a010

Browse files
fix: let PHP handle basic auth. (#2142)
I noticed that PHP likes to handle and free basic auth parameters internally (see [here](https://github.com/php/php-src/blob/9f774e3a85d34f4aa1558613a9df7703ad3bb513/main/main.c#L2739) and [here](https://github.com/php/php-src/blob/9f774e3a85d34f4aa1558613a9df7703ad3bb513/main/SAPI.c#L514-L525)). This PR changes it so the basic auth header is forwarded to PHP instead of resolving it in go. I suspect that this might fix some crashes in shutdown functions (like #2121 and #1841) since it allows us freeing the `request_info` after shutdown is finished. I haven't been able to reproduce these crashes yet though.
1 parent 07518a7 commit 0c2a010

File tree

2 files changed

+17
-19
lines changed

2 files changed

+17
-19
lines changed

cgi.go

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -277,23 +277,13 @@ func splitPos(path string, splitPath []string) int {
277277
// See: https://github.com/php/php-src/blob/345e04b619c3bc11ea17ee02cdecad6ae8ce5891/main/SAPI.h#L72
278278
//
279279
//export go_update_request_info
280-
func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info) {
280+
func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info) *C.char {
281281
thread := phpThreads[threadIndex]
282282
fc := thread.frankenPHPContext()
283283
request := fc.request
284284

285285
if request == nil {
286-
return
287-
}
288-
289-
authUser, authPassword, ok := request.BasicAuth()
290-
if ok {
291-
if authPassword != "" {
292-
info.auth_password = thread.pinCString(authPassword)
293-
}
294-
if authUser != "" {
295-
info.auth_user = thread.pinCString(authUser)
296-
}
286+
return nil
297287
}
298288

299289
info.request_method = thread.pinCString(request.Method)
@@ -311,6 +301,13 @@ func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info)
311301
info.request_uri = thread.pinCString(request.URL.RequestURI())
312302

313303
info.proto_num = C.int(request.ProtoMajor*1000 + request.ProtoMinor)
304+
305+
authorizationHeader := request.Header.Get("Authorization")
306+
if authorizationHeader == "" {
307+
return nil
308+
}
309+
310+
return thread.pinCString(authorizationHeader)
314311
}
315312

316313
// SanitizedPathJoin performs filepath.Join(root, reqPath) that

frankenphp.c

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,11 @@ static void frankenphp_update_request_context() {
8585
/* status It is not reset by zend engine, set it to 200. */
8686
SG(sapi_headers).http_response_code = 200;
8787

88-
go_update_request_info(thread_index, &SG(request_info));
88+
char *authorization_header =
89+
go_update_request_info(thread_index, &SG(request_info));
90+
91+
/* let PHP handle basic auth */
92+
php_handle_auth_data(authorization_header);
8993
}
9094

9195
static void frankenphp_free_request_context() {
@@ -95,8 +99,6 @@ static void frankenphp_free_request_context() {
9599
}
96100

97101
/* freed via thread.Unpin() */
98-
SG(request_info).auth_password = NULL;
99-
SG(request_info).auth_user = NULL;
100102
SG(request_info).request_method = NULL;
101103
SG(request_info).query_string = NULL;
102104
SG(request_info).content_type = NULL;
@@ -187,9 +189,9 @@ static void frankenphp_worker_request_shutdown() {
187189
zend_end_try();
188190

189191
/* SAPI related shutdown (free stuff) */
190-
frankenphp_free_request_context();
191192
zend_try { sapi_deactivate(); }
192193
zend_end_try();
194+
frankenphp_free_request_context();
193195

194196
zend_set_memory_limit(PG(memory_limit));
195197
}
@@ -609,8 +611,8 @@ static zend_module_entry frankenphp_module = {
609611
STANDARD_MODULE_PROPERTIES};
610612

611613
static void frankenphp_request_shutdown() {
612-
frankenphp_free_request_context();
613614
php_request_shutdown((void *)0);
615+
frankenphp_free_request_context();
614616
}
615617

616618
static int frankenphp_startup(sapi_module_struct *sapi_module) {
@@ -1055,8 +1057,7 @@ static int frankenphp_request_startup() {
10551057
return SUCCESS;
10561058
}
10571059

1058-
frankenphp_free_request_context();
1059-
php_request_shutdown((void *)0);
1060+
frankenphp_request_shutdown();
10601061

10611062
return FAILURE;
10621063
}

0 commit comments

Comments
 (0)