Webhook for Task with "description" fails

dcolley commented 3 years ago

Description of the issue

Webhook that triggers Task fails with description field

Context information (for bug reports)

Output of bench version n/a

Steps to reproduce the issue

Create a webhook for Task (any event)
put {"description": "{{doc.description}}"} in the message format
trigger a change on any task

Observed result

Expected result

No error, webhook should send data successfully

Stacktrace / full error message

Traceback (most recent call last):
  File "/home/frappe/frappe-bench/apps/frappe/frappe/utils/background_jobs.py", line 104, in execute_job
    method(**kwargs)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/integrations/doctype/webhook/webhook.py", line 83, in enqueue_webhook
    data = get_webhook_data(doc, webhook)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/integrations/doctype/webhook/webhook.py", line 144, in get_webhook_data
    data = json.loads(data)
  File "/usr/lib/python3.7/json/__init__.py", line 348, in loads
    return _default_decoder.decode(s)
  File "/usr/lib/python3.7/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/lib/python3.7/json/decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 23 column 21 (char 512)

Additional information

using all the other fields works

{
    "name": "{{doc.name}}",
    "subject": "{{doc.subject}}",
    "project": "{{doc.project}}",
    "issue": "{{doc.issue}}",
    "type": "{{doc.type}}",
    "color": "{{doc.color}}",
    "is_group": "{{doc.is_group}}",
    "is_template": "{{doc.is_template}}",
    "status": "{{doc.status}}",
    "priority": "{{doc.priority}}",
    "task_weight": "{{doc.task_weight}}",
    "parent_task": "{{doc.parent_task}}",
    "completed_by": "{{doc.completed_by}}",
    "completed_on": "{{doc.completed_on}}",
    "exp_start_date": "{{doc.exp_start_date}}",
    "expected_time": "{{doc.expected_time}}",
    "start": "{{doc.start}}",
    "exp_end_date": "{{doc.exp_end_date}}",
    "progress": "{{doc.progress}}",
    "duration": "{{doc.duration}}",
    "is_milestone": "{{doc.is_milestone}}",

    "depends_on": "{{doc.depends_on}}",
    "depands_on_tasks": "{{doc.depands_on_tasks}}",
    "act_start_date": "{{doc.act_start_date}}",
    "actual_time": "{{doc.actual_time}}",
    "act_end_date": "{{doc.act_end_date}}",
    "total_costing_amount": "{{doc.total_costing_amount}}",
    "total_expense_claim": "{{doc.total_expense_claim}}",
    "total_billing_amount": "{{doc.total_billing_amount}}",
    "review_date": "{{doc.statreview_dates}}",
    "closing_date": "{{doc.closing_date}}",
    "department": "{{doc.department}}",
    "company": "{{doc.company}}",
    "lft": "{{doc.lft}}",
    "rgt": "{{doc.rgt}}",
    "old_parent": "{{doc.old_parent}}"
}

I can call the API to get the task details: GET https://my-company.frappe.cloud/api/resource/Task/TASK-2021-00004/

{
    "data": {
        "name": "TASK-2021-00004",
        "owner": "derek.colley@...",
        "creation": "2021-11-12 14:30:37.812948",
        "modified": "2021-11-12 14:34:16.461814",
        "modified_by": "derek.colley@...",
        "idx": 0,
        "docstatus": 0,
        "subject": "Test Task 2",
        "type": "Test",
        "is_group": 0,
        "is_template": 0,
        "status": "Working",
        "priority": "Medium",
        "task_weight": 0.0,
        "exp_start_date": "2021-11-12",
        "expected_time": 40.0,
        "start": 0,
        "exp_end_date": "2021-11-19",
        "progress": 5.0,
        "duration": 0,
        "is_milestone": 0,
>>>     "description": "<div class=\"ql-editor read-mode\"><p>This is a test task. u</p></div>",   <<<
        "depends_on_tasks": "TASK-2021-00001,",
        "actual_time": 0.0,
        "total_costing_amount": 0.0,
        "total_expense_claim": 0.0,
        "total_billing_amount": 0.0,
        "company": "my company",
        "lft": 5,
        "rgt": 6,
        "old_parent": "",
        "doctype": "Task",
        "depends_on": [
            {
                "name": "2b42b1502d",
                "owner": "derek.colley@...,
                "creation": "2021-11-12 14:30:37.812948",
                "modified": "2021-11-12 14:34:16.461814",
                "modified_by": "derek.colley@...",
                "parent": "TASK-2021-00004",
                "parentfield": "depends_on",
                "parenttype": "Task",
                "idx": 1,
                "docstatus": 0,
                "task": "TASK-2021-00001",
                "doctype": "Task Depends On"
            }
        ]
    }
}

So, I suspect the problem is with escaping the HTML text in the description

OS version / distribution, `ERPNext` install method, etc.

Frappe.cloud

ankush commented 3 years ago

Just use whitelisted utility function for stripping HTML? it's kinda hard to automatically detect this.

ankush commented 3 years ago

So the reason is you've not created a correct JSON string, there's not much Framework can do about this. Even if we tried to escape it, it's still ugly HTML. Best option for you is to strip HTML. There are strip_html, html2text and escape_html whitelisted utility methods.

dcolley commented 2 years ago

@ankush thanks for your answer. Is there any documentation on how to use these methods please?

I tried the following:

{
    "name": "{{doc.name}}",
    ...
    "is_milestone": "{{doc.is_milestone}}",
    "description": "{{strip_html(doc.description)}}",
    "depends_on": "{{doc.depends_on}}",
    ...
}

But the method is not found

Traceback (most recent call last):
  File "/home/frappe/frappe-bench/apps/frappe/frappe/utils/jinja.py", line 86, in render_template
    return get_jenv().from_string(template).render(context)
  File "/home/frappe/frappe-bench/env/lib/python3.7/site-packages/jinja2/environment.py", line 1090, in render
    self.environment.handle_exception()
  File "/home/frappe/frappe-bench/env/lib/python3.7/site-packages/jinja2/environment.py", line 832, in handle_exception
    reraise(*rewrite_traceback_stack(source=source))
  File "/home/frappe/frappe-bench/env/lib/python3.7/site-packages/jinja2/_compat.py", line 28, in reraise
    raise value.with_traceback(tb)
  File "<template>", line 26, in top-level template code
  File "/home/frappe/frappe-bench/env/lib/python3.7/site-packages/jinja2/sandbox.py", line 460, in call
    if not __self.is_safe_callable(__obj):
  File "/home/frappe/frappe-bench/env/lib/python3.7/site-packages/jinja2/sandbox.py", line 360, in is_safe_callable
    getattr(obj, "unsafe_callable", False) or getattr(obj, "alters_data", False)
jinja2.exceptions.UndefinedError: 'strip_html' is undefined

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/frappe/frappe-bench/apps/frappe/frappe/utils/background_jobs.py", line 104, in execute_job
    method(**kwargs)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/integrations/doctype/webhook/webhook.py", line 83, in enqueue_webhook
    data = get_webhook_data(doc, webhook)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/integrations/doctype/webhook/webhook.py", line 143, in get_webhook_data
    data = frappe.render_template(webhook.webhook_json, get_context(doc))
  File "/home/frappe/frappe-bench/apps/frappe/frappe/utils/jinja.py", line 88, in render_template
    throw(title="Jinja Template Error", msg="<pre>{template}</pre><pre>{tb}</pre>".format(template=template, tb=get_traceback()))
  File "/home/frappe/frappe-bench/apps/frappe/frappe/__init__.py", line 438, in throw
    msgprint(msg, raise_exception=exc, title=title, indicator='red', is_minimizable=is_minimizable, wide=wide, as_list=as_list)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/__init__.py", line 417, in msgprint
    _raise_exception()
  File "/home/frappe/frappe-bench/apps/frappe/frappe/__init__.py", line 371, in _raise_exception
    raise raise_exception(msg)
frappe.exceptions.ValidationError: <pre>{
    "name": "{{doc.name}}",
    "id": "{{doc.id}}",
...
 "description": "{{strip_html(doc.description)}}",
...
   "rgt": "{{doc.rgt}}",
    "old_parent": "{{doc.old_parent}}"
}</pre><pre>Traceback (most recent call last):
  File "/home/frappe/frappe-bench/apps/frappe/frappe/utils/jinja.py", line 86, in render_template
    return get_jenv().from_string(template).render(context)
  File "/home/frappe/frappe-bench/env/lib/python3.7/site-packages/jinja2/environment.py", line 1090, in render
    self.environment.handle_exception()
  File "/home/frappe/frappe-bench/env/lib/python3.7/site-packages/jinja2/environment.py", line 832, in handle_exception
    reraise(*rewrite_traceback_stack(source=source))
  File "/home/frappe/frappe-bench/env/lib/python3.7/site-packages/jinja2/_compat.py", line 28, in reraise
    raise value.with_traceback(tb)
  File "<template>", line 26, in top-level template code
  File "/home/frappe/frappe-bench/env/lib/python3.7/site-packages/jinja2/sandbox.py", line 460, in call
    if not __self.is_safe_callable(__obj):
  File "/home/frappe/frappe-bench/env/lib/python3.7/site-packages/jinja2/sandbox.py", line 360, in is_safe_callable
    getattr(obj, "unsafe_callable", False) or getattr(obj, "alters_data", False)
jinja2.exceptions.UndefinedError: 'strip_html' is undefined
</pre>

ankush commented 2 years ago

try frappe.utils.strip_html

frappe / erpnext