diff --git a/pkgs/clan-app/clan_app/deps/http/http_bridge.py b/pkgs/clan-app/clan_app/deps/http/http_bridge.py index 4ad6d7164..b3949fe87 100644 --- a/pkgs/clan-app/clan_app/deps/http/http_bridge.py +++ b/pkgs/clan-app/clan_app/deps/http/http_bridge.py @@ -36,46 +36,151 @@ class HttpBridge(ApiBridge, BaseHTTPRequestHandler): openapi_file: Path | None = None, swagger_dist: Path | None = None, ) -> None: - # Initialize the API bridge fields + # Initialize API bridge fields self.api = api - self.openapi_file = openapi_file - self.swagger_dist = swagger_dist self.middleware_chain = middleware_chain self.threads: dict[str, WebThread] = {} - # Initialize the HTTP handler + # Initialize OpenAPI/Swagger fields + self.openapi_file = openapi_file + self.swagger_dist = swagger_dist + + # Initialize HTTP handler super(BaseHTTPRequestHandler, self).__init__(request, client_address, server) + def _send_cors_headers(self) -> None: + """Send CORS headers for cross-origin requests.""" + self.send_header("Access-Control-Allow-Origin", "*") + self.send_header("Access-Control-Allow-Methods", "GET, POST, OPTIONS") + self.send_header("Access-Control-Allow-Headers", "Content-Type") + + def _send_json_response_with_status( + self, data: dict[str, Any], status_code: int = 200 + ) -> None: + """Send a JSON response with the given status code.""" + try: + self.send_response_only(status_code) + self.send_header("Content-Type", "application/json") + self._send_cors_headers() + self.end_headers() + + response_data = json.dumps(data, indent=2, ensure_ascii=False) + self.wfile.write(response_data.encode("utf-8")) + except BrokenPipeError as e: + log.warning(f"Client disconnected before we could send a response: {e!s}") + def send_api_response(self, response: BackendResponse) -> None: """Send HTTP response directly to the client.""" + response_dict = dataclass_to_dict(response) + self._send_json_response_with_status(response_dict, 200) + log.debug( + f"HTTP response for {response._op_key}: {json.dumps(response_dict, indent=2)}" + ) # noqa: SLF001 + + def _create_success_response( + self, op_key: str, data: dict[str, Any] + ) -> BackendResponse: + """Create a successful API response.""" + return BackendResponse( + body=SuccessDataClass(op_key=op_key, status="success", data=data), + header={}, + _op_key=op_key, + ) + + def _send_info_response(self) -> None: + """Send server information response.""" + response = self._create_success_response( + "info", {"message": "Clan API Server", "version": "1.0.0"} + ) + self.send_api_response(response) + + def _send_methods_response(self) -> None: + """Send available API methods response.""" + response = self._create_success_response( + "methods", {"methods": list(self.api.functions.keys())} + ) + self.send_api_response(response) + + def _handle_swagger_request(self, parsed_url: Any) -> None: + """Handle Swagger UI related requests.""" + if not self.swagger_dist or not self.swagger_dist.exists(): + self.send_error(404, "Swagger file not found") + return + + rel_path = parsed_url.path[len("/api/swagger") :].lstrip("/") + + # Redirect /api/swagger to /api/swagger/index.html + if rel_path == "": + self.send_response(302) + self.send_header("Location", "/api/swagger/index.html") + self.end_headers() + return + + self._serve_swagger_file(rel_path) + + def _serve_swagger_file(self, rel_path: str) -> None: + """Serve a specific Swagger UI file.""" + file_path = self._get_swagger_file_path(rel_path) + + if not file_path.exists() or not file_path.is_file(): + self.send_error(404, "Swagger file not found") + return try: - # Send HTTP response - self.send_response_only(200) - self.send_header("Content-Type", "application/json") - self.send_header("Access-Control-Allow-Origin", "*") - self.send_header("Access-Control-Allow-Methods", "GET, POST, OPTIONS") - self.send_header("Access-Control-Allow-Headers", "Content-Type") + content_type = self._get_content_type(file_path) + file_data = self._read_and_process_file(file_path, rel_path) + + self.send_response(200) + self.send_header("Content-Type", content_type) self.end_headers() + self.wfile.write(file_data) + except Exception as e: + log.error(f"Error reading Swagger file: {e!s}") + self.send_error(500, "Internal Server Error") - # Write response data - response_data = json.dumps( - dataclass_to_dict(response), indent=2, ensure_ascii=False - ) - self.wfile.write(response_data.encode("utf-8")) + def _get_swagger_file_path(self, rel_path: str) -> Path: + """Get the file path for a Swagger resource.""" + if rel_path == "index.html": + return Path(__file__).parent / "swagger.html" + if rel_path == "openapi.json": + if not self.openapi_file: + return Path("/nonexistent") # Will fail exists() check + return self.openapi_file + return ( + self.swagger_dist / rel_path if self.swagger_dist else Path("/nonexistent") + ) - # Log the response for debugging - log.debug(f"HTTP response for {response._op_key}: {response_data}") # noqa: SLF001 - except BrokenPipeError as e: - # Handle broken pipe errors gracefully - log.warning(f"Client disconnected before we could send a response: {e!s}") + def _get_content_type(self, file_path: Path) -> str: + """Get the content type for a file based on its extension.""" + content_types = { + ".html": "text/html", + ".js": "application/javascript", + ".css": "text/css", + ".json": "application/json", + ".png": "image/png", + ".svg": "image/svg+xml", + } + return content_types.get(file_path.suffix, "application/octet-stream") + + def _read_and_process_file(self, file_path: Path, rel_path: str) -> bytes: + """Read and optionally process a file (e.g., inject server URL into openapi.json).""" + with file_path.open("rb") as f: + file_data = f.read() + + if rel_path == "openapi.json": + json_data = json.loads(file_data.decode("utf-8")) + server_address = getattr(self.server, "server_address", ("localhost", 80)) + json_data["servers"] = [ + {"url": f"http://{server_address[0]}:{server_address[1]}/api/v1/"} + ] + file_data = json.dumps(json_data, indent=2).encode("utf-8") + + return file_data def do_OPTIONS(self) -> None: # noqa: N802 """Handle CORS preflight requests.""" self.send_response_only(200) - self.send_header("Access-Control-Allow-Origin", "*") - self.send_header("Access-Control-Allow-Methods", "GET, POST, OPTIONS") - self.send_header("Access-Control-Allow-Headers", "Content-Type") + self._send_cors_headers() self.end_headers() def do_GET(self) -> None: # noqa: N802 @@ -84,102 +189,28 @@ class HttpBridge(ApiBridge, BaseHTTPRequestHandler): path = parsed_url.path if path == "/": - response = BackendResponse( - body=SuccessDataClass( - op_key="info", - status="success", - data={"message": "Clan API Server", "version": "1.0.0"}, - ), - header={}, - _op_key="info", - ) - self.send_api_response(response) + self._send_info_response() elif path.startswith("/api/swagger"): - if self.swagger_dist and self.swagger_dist.exists(): - # Serve static files from swagger_dist - rel_path = parsed_url.path[len("/api/swagger") :].lstrip("/") - # Redirect /api/swagger (no trailing slash or file) to /api/swagger/index.html - if rel_path == "": - self.send_response(302) - self.send_header("Location", "/api/swagger/index.html") - self.end_headers() - return - file_path = self.swagger_dist / rel_path - if rel_path == "index.html": - file_path = Path(__file__).parent / "swagger.html" - elif rel_path == "openapi.json": - if not self.openapi_file: - self.send_error(404, "OpenAPI file not found") - return - file_path = self.openapi_file - - if file_path.exists() and file_path.is_file(): - try: - # Guess content type - if file_path.suffix == ".html": - content_type = "text/html" - elif file_path.suffix == ".js": - content_type = "application/javascript" - elif file_path.suffix == ".css": - content_type = "text/css" - elif file_path.suffix == ".json": - content_type = "application/json" - elif file_path.suffix == ".png": - content_type = "image/png" - elif file_path.suffix == ".svg": - content_type = "image/svg+xml" - else: - content_type = "application/octet-stream" - with file_path.open("rb") as f: - file_data = f.read() - if rel_path == "openapi.json": - json_data = json.loads(file_data.decode("utf-8")) - json_data["servers"] = [ - { - "url": f"http://{getattr(self.server, 'server_address', ('localhost', 80))[0]}:{getattr(self.server, 'server_address', ('localhost', 80))[1]}/api/v1/" - } - ] - file_data = json.dumps(json_data, indent=2).encode("utf-8") - self.send_response(200) - self.send_header("Content-Type", content_type) - self.end_headers() - - self.wfile.write(file_data) - except Exception as e: - log.error(f"Error reading Swagger file: {e!s}") - self.send_error(500, "Internal Server Error") - else: - self.send_error(404, "Swagger file not found") - else: - self.send_error(404, "Swagger file not found") - + self._handle_swagger_request(parsed_url) elif path == "/api/methods": - response = BackendResponse( - body=SuccessDataClass( - op_key="methods", - status="success", - data={"methods": list(self.api.functions.keys())}, - ), - header={}, - _op_key="methods", - ) - self.send_api_response(response) + self._send_methods_response() + else: + self.send_api_error_response("info", "Not Found", ["http_bridge", "GET"]) def do_POST(self) -> None: # noqa: N802 """Handle POST requests.""" parsed_url = urlparse(self.path) path = parsed_url.path - # Check if this is an API call + # Validate API path if not path.startswith("/api/v1/"): self.send_api_error_response( - "post", f"Path not found {path}", ["http_bridge", "POST"] + "post", f"Path not found: {path}", ["http_bridge", "POST"] ) return - # Extract method name from path + # Extract and validate method name method_name = path[len("/api/v1/") :] - if not method_name: self.send_api_error_response( "post", "Method name required", ["http_bridge", "POST"] @@ -194,33 +225,13 @@ class HttpBridge(ApiBridge, BaseHTTPRequestHandler): ) return - # Read request body - try: - content_length = int(self.headers.get("Content-Length", 0)) - if content_length > 0: - body = self.rfile.read(content_length) - request_data = json.loads(body.decode("utf-8")) - else: - request_data = {} - except json.JSONDecodeError: - self.send_api_error_response( - "post", - "Invalid JSON in request body", - ["http_bridge", "POST", method_name], - ) - return - except Exception as e: - self.send_api_error_response( - "post", - f"Error reading request: {e!s}", - ["http_bridge", "POST", method_name], - ) - return + # Read and parse request body + request_data = self._read_request_body(method_name) + if request_data is None: + return # Error already sent - # Generate a unique operation key + # Generate operation key and handle request gen_op_key = str(uuid.uuid4()) - - # Handle the API request try: self._handle_api_request(method_name, request_data, gen_op_key) except Exception as e: @@ -231,6 +242,29 @@ class HttpBridge(ApiBridge, BaseHTTPRequestHandler): ["http_bridge", "POST", method_name], ) + def _read_request_body(self, method_name: str) -> dict[str, Any] | None: + """Read and parse the request body. Returns None if there was an error.""" + try: + content_length = int(self.headers.get("Content-Length", 0)) + if content_length > 0: + body = self.rfile.read(content_length) + return json.loads(body.decode("utf-8")) + return {} + except json.JSONDecodeError: + self.send_api_error_response( + "post", + "Invalid JSON in request body", + ["http_bridge", "POST", method_name], + ) + return None + except Exception as e: + self.send_api_error_response( + "post", + f"Error reading request: {e!s}", + ["http_bridge", "POST", method_name], + ) + return None + def _handle_api_request( self, method_name: str, @@ -239,32 +273,11 @@ class HttpBridge(ApiBridge, BaseHTTPRequestHandler): ) -> None: """Handle an API request by processing it through middleware.""" try: - # Parse the HTTP request format - header = request_data.get("header", {}) - if not isinstance(header, dict): - msg = f"Expected header to be a dict, got {type(header)}" - raise TypeError(msg) + # Validate and parse request data + header, body, op_key = self._parse_request_data(request_data, gen_op_key) - body = request_data.get("body", {}) - if not isinstance(body, dict): - msg = f"Expected body to be a dict, got {type(body)}" - raise TypeError(msg) - - op_key = header.get("op_key", gen_op_key) - if not isinstance(op_key, str): - msg = f"Expected op_key to be a string, got {type(op_key)}" - raise TypeError(msg) - - # Check if op_key is a valid UUID - try: - uuid.UUID(op_key) - except ValueError as e: - msg = f"op_key '{op_key}' is not a valid UUID" - raise TypeError(msg) from e - - if op_key in self.threads: - msg = f"Operation key '{op_key}' is already in use. Please try again." - raise ValueError(msg) + # Validate operation key + self._validate_operation_key(op_key) # Create API request api_request = BackendRequest( @@ -272,14 +285,54 @@ class HttpBridge(ApiBridge, BaseHTTPRequestHandler): ) except Exception as e: - # Create error response directly - self.send_api_error_response(op_key, str(e), ["http_bridge", method_name]) + self.send_api_error_response( + gen_op_key, str(e), ["http_bridge", method_name] + ) return - # Process in a separate thread + self._process_api_request_in_thread(api_request, method_name) + + def _parse_request_data( + self, request_data: dict[str, Any], gen_op_key: str + ) -> tuple[dict[str, Any], dict[str, Any], str]: + """Parse and validate request data components.""" + header = request_data.get("header", {}) + if not isinstance(header, dict): + msg = f"Expected header to be a dict, got {type(header)}" + raise TypeError(msg) + + body = request_data.get("body", {}) + if not isinstance(body, dict): + msg = f"Expected body to be a dict, got {type(body)}" + raise TypeError(msg) + + op_key = header.get("op_key", gen_op_key) + if not isinstance(op_key, str): + msg = f"Expected op_key to be a string, got {type(op_key)}" + raise TypeError(msg) + + return header, body, op_key + + def _validate_operation_key(self, op_key: str) -> None: + """Validate that the operation key is valid and not in use.""" + try: + uuid.UUID(op_key) + except ValueError as e: + msg = f"op_key '{op_key}' is not a valid UUID" + raise TypeError(msg) from e + + if op_key in self.threads: + msg = f"Operation key '{op_key}' is already in use. Please try again." + raise ValueError(msg) + + def _process_api_request_in_thread( + self, api_request: BackendRequest, method_name: str + ) -> None: + """Process the API request in a separate thread.""" + op_key = api_request.op_key or "unknown" + def thread_task(stop_event: threading.Event) -> None: set_should_cancel(lambda: stop_event.is_set()) - try: self.process_request(api_request) finally: @@ -295,7 +348,7 @@ class HttpBridge(ApiBridge, BaseHTTPRequestHandler): # Wait for the thread to complete (this blocks until response is sent) thread.join(timeout=60.0) - # If thread is still alive, it timed out + # Handle timeout if thread.is_alive(): stop_event.set() # Cancel the thread self.send_api_error_response( diff --git a/pkgs/clan-app/clan_app/deps/http/test_http_api.py b/pkgs/clan-app/clan_app/deps/http/test_http_api.py index eb4cb0ed2..753f6afcb 100644 --- a/pkgs/clan-app/clan_app/deps/http/test_http_api.py +++ b/pkgs/clan-app/clan_app/deps/http/test_http_api.py @@ -147,7 +147,7 @@ class TestHttpApiServer: # Test API call endpoint request_data: dict = {"header": {}, "body": {"message": "World"}} req: Request = Request( - "http://127.0.0.1:8081/api/call/test_method", + "http://127.0.0.1:8081/api/v1/test_method", data=json.dumps(request_data).encode(), headers={"Content-Type": "application/json"}, ) @@ -182,7 +182,7 @@ class TestHttpApiServer: # Test method not found request_data: dict = {"header": {}, "body": {}} req: Request = Request( - "http://127.0.0.1:8081/api/call/nonexistent_method", + "http://127.0.0.1:8081/api/v1/nonexistent_method", data=json.dumps(request_data).encode(), headers={"Content-Type": "application/json"}, ) @@ -194,7 +194,7 @@ class TestHttpApiServer: # Test invalid JSON req = Request( - "http://127.0.0.1:8081/api/call/test_method", + "http://127.0.0.1:8081/api/v1/test_method", data=b"invalid json", headers={"Content-Type": "application/json"}, ) @@ -264,7 +264,7 @@ class TestIntegration: "body": {"message": "Integration"}, } req: Request = Request( - "http://127.0.0.1:8082/api/call/test_method", + "http://127.0.0.1:8082/api/v1/test_method", data=json.dumps(request_data).encode(), headers={"Content-Type": "application/json"}, ) diff --git a/pyproject.toml b/pyproject.toml index 108f5cf7e..6abac8cd7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -50,6 +50,7 @@ lint.ignore = [ # We might actually want to fix this. "A005", "TRY301", + "TRY300", "ANN401", "RUF100", "TRY400",