Skip to content

Commit 4dad5ed

Browse files
committed
Fix ASGI scope caching bug: treat method as dynamic field
The scope cache was keyed by path but method was not in the dynamic fields list. This caused incorrect method values when the same path was accessed with different HTTP methods (e.g., GET /path then POST /path would return method="GET" for both). Changes: - Add ATOM_ASGI_METHOD for efficient map lookup - Update method in update_dynamic_scope_fields() - Remove method from cached template in get_cached_scope() - Add unit test to verify method caching behavior
1 parent 4050475 commit 4dad5ed

4 files changed

Lines changed: 80 additions & 1 deletion

File tree

c_src/py_asgi.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ ERL_NIF_TERM ATOM_ASGI_PATH;
5555
ERL_NIF_TERM ATOM_ASGI_HEADERS;
5656
ERL_NIF_TERM ATOM_ASGI_CLIENT;
5757
ERL_NIF_TERM ATOM_ASGI_QUERY_STRING;
58+
ERL_NIF_TERM ATOM_ASGI_METHOD;
5859

5960
/* Resource type for zero-copy body buffers */
6061
ErlNifResourceType *ASGI_BUFFER_RESOURCE_TYPE = NULL;
@@ -1128,7 +1129,7 @@ static void asgi_cleanup_scope_cache(void) {
11281129
/**
11291130
* @brief Update dynamic fields in a cloned scope
11301131
*
1131-
* Updates client, headers, and query_string which vary per request.
1132+
* Updates client, headers, query_string, and method which vary per request.
11321133
*/
11331134
static int update_dynamic_scope_fields(ErlNifEnv *env, PyObject *scope,
11341135
ERL_NIF_TERM scope_map) {
@@ -1237,6 +1238,20 @@ static int update_dynamic_scope_fields(ErlNifEnv *env, PyObject *scope,
12371238
}
12381239
}
12391240

1241+
/* Update method - use Erlang atom for map lookup */
1242+
if (enif_get_map_value(env, scope_map, ATOM_ASGI_METHOD, &value)) {
1243+
ErlNifBinary method_bin;
1244+
if (enif_inspect_binary(env, value, &method_bin)) {
1245+
PyObject *py_method = asgi_get_method((char *)method_bin.data, method_bin.size);
1246+
if (py_method == NULL) return -1;
1247+
if (PyDict_SetItem(scope, state->key_method, py_method) < 0) {
1248+
Py_DECREF(py_method);
1249+
return -1;
1250+
}
1251+
Py_DECREF(py_method);
1252+
}
1253+
}
1254+
12401255
return 0;
12411256
}
12421257

@@ -2378,6 +2393,7 @@ static PyObject *get_cached_scope(ErlNifEnv *env, ERL_NIF_TERM scope_map) {
23782393
PyDict_DelItem(template, state->key_client);
23792394
PyDict_DelItem(template, state->key_headers);
23802395
PyDict_DelItem(template, state->key_query_string);
2396+
PyDict_DelItem(template, state->key_method);
23812397
PyErr_Clear(); /* DelItem may fail if key doesn't exist */
23822398

23832399
/* If replacing entry from different interpreter, release old reference

c_src/py_asgi.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ extern ERL_NIF_TERM ATOM_ASGI_PATH;
115115
extern ERL_NIF_TERM ATOM_ASGI_HEADERS;
116116
extern ERL_NIF_TERM ATOM_ASGI_CLIENT;
117117
extern ERL_NIF_TERM ATOM_ASGI_QUERY_STRING;
118+
extern ERL_NIF_TERM ATOM_ASGI_METHOD;
118119

119120
/* Resource type for zero-copy body buffers */
120121
extern ErlNifResourceType *ASGI_BUFFER_RESOURCE_TYPE;

c_src/py_nif.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1782,6 +1782,7 @@ static int load(ErlNifEnv *env, void **priv_data, ERL_NIF_TERM load_info) {
17821782
ATOM_ASGI_HEADERS = enif_make_atom(env, "headers");
17831783
ATOM_ASGI_CLIENT = enif_make_atom(env, "client");
17841784
ATOM_ASGI_QUERY_STRING = enif_make_atom(env, "query_string");
1785+
ATOM_ASGI_METHOD = enif_make_atom(env, "method");
17851786

17861787
/* ASGI buffer resource type for zero-copy body handling */
17871788
ASGI_BUFFER_RESOURCE_TYPE = enif_open_resource_type(

test/py_SUITE.erl

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
test_asgi_header_caching/1,
5454
test_asgi_status_codes/1,
5555
test_asgi_scope_caching/1,
56+
test_asgi_scope_method_caching/1,
5657
test_asgi_zero_copy_buffer/1,
5758
test_asgi_lazy_headers/1
5859
]).
@@ -102,6 +103,7 @@ all() ->
102103
test_asgi_header_caching,
103104
test_asgi_status_codes,
104105
test_asgi_scope_caching,
106+
test_asgi_scope_method_caching,
105107
test_asgi_zero_copy_buffer,
106108
test_asgi_lazy_headers
107109
].
@@ -1045,6 +1047,65 @@ test_asgi_scope_caching(_Config) ->
10451047
ct:pal("Scope caching test passed~n"),
10461048
ok.
10471049

1050+
%% Test that method is correctly updated when scope cache is hit
1051+
%% This verifies the fix for the bug where method was cached with the path
1052+
%% causing GET /path followed by POST /path to return method="GET"
1053+
test_asgi_scope_method_caching(_Config) ->
1054+
%% This test uses the NIF directly to verify scope caching behavior.
1055+
%% The scope cache keys on path, so we need to verify that method
1056+
%% is treated as a dynamic field and updated on cache hits.
1057+
1058+
%% Build first scope with GET method
1059+
Scope1Map = #{
1060+
type => <<"http">>,
1061+
asgi => #{version => <<"3.0">>, spec_version => <<"2.3">>},
1062+
http_version => <<"1.1">>,
1063+
method => <<"GET">>,
1064+
scheme => <<"http">>,
1065+
path => <<"/api/test">>,
1066+
raw_path => <<"/api/test">>,
1067+
query_string => <<>>,
1068+
root_path => <<>>,
1069+
headers => [{<<"host">>, <<"localhost">>}],
1070+
server => {<<"localhost">>, 8080},
1071+
client => {<<"127.0.0.1">>, 12345}
1072+
},
1073+
{ok, ScopeRef1} = py_nif:asgi_build_scope(Scope1Map),
1074+
1075+
%% Build second scope with POST method on same path (should hit cache)
1076+
Scope2Map = Scope1Map#{
1077+
method => <<"POST">>,
1078+
headers => [{<<"content-type">>, <<"application/json">>}],
1079+
client => {<<"127.0.0.1">>, 12346}
1080+
},
1081+
{ok, ScopeRef2} = py_nif:asgi_build_scope(Scope2Map),
1082+
1083+
%% Build third scope with PUT method on same path
1084+
Scope3Map = Scope1Map#{
1085+
method => <<"PUT">>,
1086+
headers => [{<<"content-type">>, <<"text/plain">>}],
1087+
client => {<<"127.0.0.1">>, 12347}
1088+
},
1089+
{ok, ScopeRef3} = py_nif:asgi_build_scope(Scope3Map),
1090+
1091+
%% Verify each scope has the correct method by extracting from Python dict
1092+
{ok, <<"GET">>} = py:eval(<<"scope['method']">>, #{scope => ScopeRef1}),
1093+
{ok, <<"POST">>} = py:eval(<<"scope['method']">>, #{scope => ScopeRef2}),
1094+
{ok, <<"PUT">>} = py:eval(<<"scope['method']">>, #{scope => ScopeRef3}),
1095+
1096+
%% Also verify other dynamic fields are correctly set
1097+
{ok, {<<"127.0.0.1">>, 12345}} = py:eval(<<"tuple(scope['client'])">>, #{scope => ScopeRef1}),
1098+
{ok, {<<"127.0.0.1">>, 12346}} = py:eval(<<"tuple(scope['client'])">>, #{scope => ScopeRef2}),
1099+
{ok, {<<"127.0.0.1">>, 12347}} = py:eval(<<"tuple(scope['client'])">>, #{scope => ScopeRef3}),
1100+
1101+
%% Verify static fields are shared correctly (path should be same)
1102+
{ok, <<"/api/test">>} = py:eval(<<"scope['path']">>, #{scope => ScopeRef1}),
1103+
{ok, <<"/api/test">>} = py:eval(<<"scope['path']">>, #{scope => ScopeRef2}),
1104+
{ok, <<"/api/test">>} = py:eval(<<"scope['path']">>, #{scope => ScopeRef3}),
1105+
1106+
ct:pal("Scope method caching test passed~n"),
1107+
ok.
1108+
10481109
%% Test zero-copy buffer handling for large bodies
10491110
test_asgi_zero_copy_buffer(_Config) ->
10501111
%% This test verifies that large bodies are handled correctly

0 commit comments

Comments
 (0)