From 1e9a285002173799f0505a35e62e533b21be1576 Mon Sep 17 00:00:00 2001 From: Harikrishna KP Date: Fri, 31 Jul 2026 10:34:49 +0530 Subject: [PATCH] fix(http_request): JSON-encode string variables when building a JSON body convert_template(...).text concatenates raw values into the template, so a string carrying a quote, backslash or newline breaks the surrounding JSON. repair_json then does not repair it: for the case in langgenius/dify#31927 it parses the wreckage down to an empty string, so the node silently posts "content": "" to the upstream API. The template the author wrote is valid JSON. Substitution is what breaks it, so encode each string value with json.dumps at the point of substitution. The document stays valid, no repair is needed, and the value round-trips byte for byte. A variable already inside a string literal 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 behaves the same as one standing alone. repair_json is retained as a fallback for templates that were already malformed before substitution, which escaping cannot fix. Signed-off-by: Harikrishna KP --- src/graphon/nodes/http_request/executor.py | 90 +++++++++++++++++--- tests/nodes/http_request/test_dispatch.py | 95 ++++++++++++++++++++++ 2 files changed, 175 insertions(+), 10 deletions(-) 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(