From dbc8b176b17651c843933930245f56347bcadddf Mon Sep 17 00:00:00 2001 From: Sogoss <> Date: Mon, 20 Jul 2026 16:03:29 +0200 Subject: [PATCH] fix(nfproxy): fix HttpHistory parameter ordering and state immutability - Fix parameter order sensitivity in HttpHistory._fetch_packet when history is first argument - Delegate total_size and should_upgrade to _message instead of live _parser - Sanitize FGEX_MAX_HISTORY_SIZE with max(0, int(val)) and dynamically sync deque maxlen --- fgex-lib/firegex/nfproxy/models/http.py | 38 ++++--- tests/test_http_history.py | 131 ++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 12 deletions(-) diff --git a/fgex-lib/firegex/nfproxy/models/http.py b/fgex-lib/firegex/nfproxy/models/http.py index 39f3f26..6ffcd4e 100644 --- a/fgex-lib/firegex/nfproxy/models/http.py +++ b/fgex-lib/firegex/nfproxy/models/http.py @@ -426,19 +426,21 @@ class HttpHistory: ): raise NotReadyToRun() - for key in ["http_module", "http_full", "http_header"]: + for key in ("http_module", "http_full", "http_header"): obj = internal_data.call_mem.get(f"_fetched_obj_{key}") if obj is not None: - if isinstance(obj, list) and len(obj) > 0: + if isinstance(obj, list) and obj: return [item.history for item in obj] elif hasattr(obj, "history"): return obj.history - fetch_cls = HttpRequest if internal_data.current_pkt.is_input else HttpResponse - obj = fetch_cls._fetch_packet(internal_data) - if isinstance(obj, list): - return [item.history for item in obj] - return obj.history + req_history_deque = internal_data.data_handler_context.get( + "http_history_requests", deque() + ) + resp_history_deque = internal_data.data_handler_context.get( + "http_history_responses", deque() + ) + return HttpHistory(list(req_history_deque), list(resp_history_deque)) def __repr__(self): return f"" @@ -473,8 +475,8 @@ class InternalBasicHttpMetaClass: @property def total_size(self) -> int: - """Total size of the stream""" - return self._parser.total_size + """Total size of the message""" + return self._message.total_size @property def url(self) -> str | None: @@ -524,7 +526,7 @@ class InternalBasicHttpMetaClass: @property def should_upgrade(self) -> bool: """If the message should upgrade""" - return self._parser.should_upgrade + return self._message.should_upgrade @property def content_length(self) -> int | None: @@ -690,17 +692,29 @@ class InternalBasicHttpMetaClass: if messages_to_call == 0: raise NotReadyToRun() - max_history = int(internal_data.filter_glob.get("FGEX_MAX_HISTORY_SIZE", 100)) + raw_max = internal_data.filter_glob.get("FGEX_MAX_HISTORY_SIZE", 100) + try: + max_history = max(0, int(raw_max)) + except (ValueError, TypeError): + max_history = 100 + req_history_deque: deque = ( internal_data.data_handler_context.setdefault( "http_history_requests", deque(maxlen=max_history) ) ) + if req_history_deque.maxlen != max_history: + req_history_deque = deque(req_history_deque, maxlen=max_history) + internal_data.data_handler_context["http_history_requests"] = req_history_deque + resp_history_deque: deque = ( internal_data.data_handler_context.setdefault( "http_history_responses", deque(maxlen=max_history) ) ) + if resp_history_deque.maxlen != max_history: + resp_history_deque = deque(resp_history_deque, maxlen=max_history) + internal_data.data_handler_context["http_history_responses"] = resp_history_deque built_instances = [] for msg in messages_tosend: @@ -711,7 +725,7 @@ class InternalBasicHttpMetaClass: instance._history = history_snapshot built_instances.append(instance) - if msg.message_complete and not getattr(msg, "added_to_history", False): + if msg.message_complete and not msg.added_to_history: msg.added_to_history = True if internal_data.current_pkt.is_input: full_req = HttpFullRequest(parser, msg) diff --git a/tests/test_http_history.py b/tests/test_http_history.py index 0f1cc9e..e7514e5 100644 --- a/tests/test_http_history.py +++ b/tests/test_http_history.py @@ -139,8 +139,139 @@ def filter_resp(resp: HttpFullResponse): assert lengths[2] == (1, 1) +def test_http_history_invalid_max_size(): + glob = {} + clear_pyfilter_registry() + + code = """ +from firegex.nfproxy import pyfilter, ACCEPT +from firegex.nfproxy.models import HttpFullRequest + +@pyfilter +def filter_req(req: HttpFullRequest): + return ACCEPT +""" + + glob["__firegex_pyfilter_enabled"] = ["filter_req"] + glob["__firegex_proto"] = "http" + glob["FGEX_MAX_HISTORY_SIZE"] = "invalid_number" + exec(code, glob, glob) + compile(glob) + + glob["__firegex_packet_info"] = create_packet_info(b"GET /test HTTP/1.1\r\nHost: test\r\n\r\n", is_input=True) + # Should not raise ValueError, should default to 100 + handle_packet(glob) + + +def test_http_history_parameter_order(): + glob = {} + clear_pyfilter_registry() + + code = """ +from firegex.nfproxy import pyfilter, ACCEPT +from firegex.nfproxy.models import HttpFullRequest, HttpHistory + +calls_log = [] + +@pyfilter +def filter_history_first(history: HttpHistory, req: HttpFullRequest): + calls_log.append(("history_first", req.url, len(history.requests))) + return ACCEPT +""" + + glob["__firegex_pyfilter_enabled"] = ["filter_history_first"] + glob["__firegex_proto"] = "http" + exec(code, glob, glob) + compile(glob) + + glob["__firegex_packet_info"] = create_packet_info(b"GET /first HTTP/1.1\r\nHost: test\r\n\r\n", is_input=True) + handle_packet(glob) + + calls = glob["calls_log"] + # Must be called EXACTLY ONCE with 0 history elements for the first request + assert len(calls) == 1 + assert calls[0] == ("history_first", "/first", 0) + + +def test_http_history_property_immutability(): + glob = {} + clear_pyfilter_registry() + immutability_log = [] + + code = """ +from firegex.nfproxy import pyfilter, ACCEPT +from firegex.nfproxy.models import HttpFullRequest, HttpHistory + +@pyfilter +def filter_req(req: HttpFullRequest, history: HttpHistory): + if len(history.requests) > 0: + past = history.requests[0] + immutability_log.append({ + "past_size": past.total_size, + "past_url": past.url, + "curr_size": req.total_size, + "curr_url": req.url, + }) + return ACCEPT +""" + + glob["__firegex_pyfilter_enabled"] = ["filter_req"] + glob["__firegex_proto"] = "http" + glob["immutability_log"] = immutability_log + exec(code, glob, glob) + compile(glob) + + # 1. First short request + req1_data = b"GET /short HTTP/1.1\r\nHost: test\r\n\r\n" + glob["__firegex_packet_info"] = create_packet_info(req1_data, is_input=True) + handle_packet(glob) + + # 2. Second longer request + req2_data = b"POST /large_path HTTP/1.1\r\nHost: test\r\nContent-Length: 5\r\n\r\nHELLO" + glob["__firegex_packet_info"] = create_packet_info(req2_data, is_input=True) + handle_packet(glob) + + entries = glob["immutability_log"] + assert len(entries) == 1 + # Historical request 1 size must equal 14 (/short), current size 39 (/large_path) + assert entries[0]["past_size"] == 14 + assert entries[0]["past_url"] == "/short" + assert entries[0]["curr_size"] == 39 + assert entries[0]["curr_url"] == "/large_path" + + +def test_http_history_negative_max_size(): + glob = {} + clear_pyfilter_registry() + + code = """ +from firegex.nfproxy import pyfilter, ACCEPT +from firegex.nfproxy.models import HttpFullRequest + +@pyfilter +def filter_req(req: HttpFullRequest): + return ACCEPT +""" + + glob["__firegex_pyfilter_enabled"] = ["filter_req"] + glob["__firegex_proto"] = "http" + glob["FGEX_MAX_HISTORY_SIZE"] = "-5" + exec(code, glob, glob) + compile(glob) + + glob["__firegex_packet_info"] = create_packet_info(b"GET /test HTTP/1.1\r\nHost: test\r\n\r\n", is_input=True) + # Should not raise ValueError for maxlen=-5, should clamp to 0 + handle_packet(glob) + + if __name__ == "__main__": test_http_history_model() test_http_history_stream_execution() test_http_history_attribute_on_request() + test_http_history_invalid_max_size() + test_http_history_parameter_order() + test_http_history_property_immutability() + test_http_history_negative_max_size() print("All HTTP history unit tests passed successfully!") + +