diff --git a/src/graphon/nodes/http_request/executor.py b/src/graphon/nodes/http_request/executor.py index 6a2c71d9..d088e20e 100644 --- a/src/graphon/nodes/http_request/executor.py +++ b/src/graphon/nodes/http_request/executor.py @@ -12,8 +12,8 @@ from graphon.file.enums import FileTransferMethod from graphon.http import HttpClientProtocol, HttpResponse from graphon.runtime.variable_pool import VariablePool -from graphon.variables.segments import ArrayFileSegment, FileSegment -from graphon.variables.template_resolution import convert_template +from graphon.variables.segments import ArrayFileSegment, FileSegment, StringSegment +from graphon.variables.template_resolution import VARIABLE_PATTERN, convert_template from ..protocols import FileManagerProtocol from .entities import ( @@ -34,6 +34,27 @@ ResponseSizeError, ) + +def _json_string_open_after(literal: str, inside: bool) -> bool: + """Whether a JSON string literal is still open after scanning ``literal``. + + Only backslash escapes inside a string are honoured, matching JSON's own + lexing rules, so an escaped quote does not close the string. + + Returns: + True when a string literal is still open at the end of ``literal``. + """ + escaped = False + for char in literal: + if escaped: + escaped = False + elif inside and char == "\\": + escaped = True + elif char == '"': + inside = not inside + return inside + + BODY_TYPE_TO_CONTENT_TYPE = { "json": "application/json", "x-www-form-urlencoded": "application/x-www-form-urlencoded", @@ -274,17 +295,66 @@ def _init_raw_text_body(self, data: Sequence[BodyData]) -> None: def _init_json_body(self, data: Sequence[BodyData]) -> None: item = self._require_single_body_item(data, body_type="json") - group = convert_template(self.variable_pool, item.value) - json_string = group.text + json_string = self._render_json_template(item.value) # Raw masked template text (not re-normalized via json.dumps); self.json # holds the parsed plaintext, so _log_json_text must never fall back to it. - self._log_json_text = group.log + self._log_json_text = self._render_json_template(item.value, masked=True) try: - repaired = repair_json(json_string) - self.json = json.loads(repaired, strict=False) - except json.JSONDecodeError as e: - msg = f"Failed to parse JSON: {json_string}" - raise RequestBodyError(msg) from e + self.json = json.loads(json_string, strict=False) + except json.JSONDecodeError: + # The template itself was malformed before any substitution, which + # escaping cannot fix. Fall back to the historical repair path so + # workflows that relied on it keep working. + try: + self.json = json.loads(repair_json(json_string), strict=False) + except json.JSONDecodeError as e: + msg = f"Failed to parse JSON: {json_string}" + raise RequestBodyError(msg) from e + + def _render_json_template(self, template: str, *, masked: bool = False) -> str: + """Substitute variables into a JSON body template, encoding as we go. + + ``convert_template(...).text`` concatenates raw values, so a string + holding a quote, backslash or newline breaks the surrounding JSON and + ``repair_json`` then silently discards it. Encoding each string value + with :func:`json.dumps` at the point of substitution keeps the document + valid, so no repair is needed and the value round-trips byte for byte. + + A variable already sitting inside a string literal (``"{{#x#}}"``, or + ``"a{{#x#}}b"``) gets the escaped body without the quotes ``json.dumps`` + would add. Quote state is tracked across the literal parts rather than + sniffed from the adjacent characters, so a variable embedded partway + through a string is handled the same as one standing alone. + + Returns: + The rendered JSON document. Secret values are obfuscated when + ``masked`` is set, for the log-safe copy. + """ + # VARIABLE_PATTERN captures the selector, so splitting yields + # [literal, selector, literal, selector, ..., literal]. + parts = VARIABLE_PATTERN.split(template) + rendered: list[str] = [] + inside_string = False + for index, part in enumerate(parts): + if index % 2 == 0: + inside_string = _json_string_open_after(part, inside_string) + rendered.append(part) + continue + + segment = self.variable_pool.get(part.split(".")) + if segment is None: + # Unresolved selector: preserve the historical behaviour of + # dropping it rather than failing the request. + rendered.append("") + elif isinstance(segment, StringSegment): + value = segment.log if masked else segment.text + encoded = json.dumps(value, ensure_ascii=False) + rendered.append(encoded[1:-1] if inside_string else encoded) + else: + # Numbers, booleans, objects and arrays already render as valid + # JSON tokens. + rendered.append(segment.log if masked else segment.text) + return "".join(rendered) def _init_binary_body(self, data: Sequence[BodyData]) -> None: item = self._require_single_body_item(data, body_type="binary") diff --git a/tests/nodes/http_request/test_dispatch.py b/tests/nodes/http_request/test_dispatch.py index 4625a193..12b9de41 100644 --- a/tests/nodes/http_request/test_dispatch.py +++ b/tests/nodes/http_request/test_dispatch.py @@ -1,3 +1,4 @@ +import json from unittest.mock import MagicMock from graphon.nodes.http_request.entities import ( @@ -91,6 +92,100 @@ def test_executor_initializes_json_body_with_body_handler_map() -> None: assert executor.json == {"answer": 1} +def _json_body_executor(template: str, variables: object = ()) -> Executor: + return Executor( + node_data=HttpRequestNodeData.model_validate({ + "method": "post", + "url": "https://example.com", + "authorization": {"type": "no-auth"}, + "headers": "", + "params": "", + "body": { + "type": "json", + "data": [{"type": "text", "value": template}], + }, + }), + timeout=HttpRequestNodeTimeout(connect=1, read=1, write=1), + variable_pool=build_variable_pool(variables=variables), + http_request_config=_build_http_request_config(), + http_client=MagicMock(), + file_manager=MagicMock(), + ) + + +# A Code node returning markdown: double quotes, backslashes, newlines, pipes. +_MARKDOWN = '### a\n\n| a | a\\a\\a | a : "[a.a](a://a)" | a + "&a\\b=c" |\n' + + +def test_json_body_preserves_a_string_containing_json_metacharacters() -> None: + """langgenius/dify#31927. + + The template is valid JSON; substituting a raw value into it is what + breaks the document, and repair_json then dropped the value entirely, + sending an empty string to the upstream API. + """ + template = ( + '{"model": "pro", "messages": [{"role": "user", "content": {{#node.result#}}}]}' + ) + executor = _json_body_executor( + template, + variables=[(["node", "result"], _MARKDOWN)], + ) + + assert executor.json["messages"][0]["content"] == _MARKDOWN + + +def test_json_body_preserves_a_string_the_author_already_quoted() -> None: + executor = _json_body_executor( + '{"content": "{{#node.result#}}"}', + variables=[(["node", "result"], 'he said "hi"\nbye')], + ) + + assert executor.json["content"] == 'he said "hi"\nbye' + + +def test_json_body_preserves_a_string_embedded_partway_through_a_literal() -> None: + """Quote state is tracked across literals, not sniffed from neighbours.""" + executor = _json_body_executor( + '{"content": "prefix {{#node.result#}} suffix"}', + variables=[(["node", "result"], 'a"b')], + ) + + assert executor.json["content"] == 'prefix a"b suffix' + + +def test_json_body_keeps_non_string_values_as_json_tokens() -> None: + executor = _json_body_executor( + '{"count": {{#node.n#}}, "ok": {{#node.b#}}, "obj": {{#node.o#}}}', + variables=[ + (["node", "n"], 42), + (["node", "b"], True), + (["node", "o"], {"k": "v"}), + ], + ) + + assert executor.json == {"count": 42, "ok": True, "obj": {"k": "v"}} + + +def test_json_body_still_repairs_a_template_that_was_malformed_to_begin_with() -> None: + """Escaping cannot fix a broken template, so the repair path is retained.""" + executor = _json_body_executor("{'answer': 1,}") + + assert executor.json == {"answer": 1} + + +def test_json_body_encodes_the_log_copy_the_same_way() -> None: + """The log copy must also be valid JSON, not the raw concatenation.""" + plain = 'a"b\nc' + executor = _json_body_executor( + '{"note": {{#node.value#}}}', + variables=[(["node", "value"], plain)], + ) + + assert executor.json["note"] == plain + assert json.loads(executor._log_json_text)["note"] == plain + + def test_executor_initializes_form_data_placeholder_when_no_files_resolve() -> None: file_manager = MagicMock() executor = Executor(