digitalfabrik / integreat-cms

Simplified content management back end for the Integreat App - a multilingual information platform for newcomers
https://digitalfabrik.github.io/integreat-cms/
Apache License 2.0
55 stars 33 forks source link

HTTP Error 500 from KeyError when image lacks src attribute #2724

Closed PeterNerlich closed 2 months ago

PeterNerlich commented 5 months ago

Describe the Bug

When checking the src value for images in pages that were just saved, the server assumes that this attribute exists. This is not necessarily the case.

Steps to Reproduce

  1. Go to any page of any region or go to create a new one
  2. Change the editor from WYSIWYG to HTML by clicking on the View menu and select Source view
  3. Paste in <img>
  4. Click on Update
  5. See error

Expected Behavior

The page form view loads again, and maybe displays a hint that there are empty images

Actual Behavior

The server encounters an uncaught KeyError

Additional Information

Traceback ``` Apr 02 16:03:53 ERROR django.request - 500 Internal Server Error: /augsburg/pages/de/1/edit/ Traceback (most recent call last): File "integreat-cms/.venv/lib/python3.11/site-packages/django/core/handlers/exception.py", line 55, in inner response = get_response(request) ^^^^^^^^^^^^^^^^^^^^^ File "integreat-cms/.venv/lib/python3.11/site-packages/django/core/handlers/base.py", line 197, in _get_response response = wrapped_callback(request, *callback_args, **callback_kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "integreat-cms/.venv/lib/python3.11/site-packages/django/views/generic/base.py", line 104, in view return self.dispatch(request, *args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "integreat-cms/.venv/lib/python3.11/site-packages/django/utils/decorators.py", line 46, in _wrapper return bound_method(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "integreat-cms/.venv/lib/python3.11/site-packages/django/contrib/auth/decorators.py", line 23, in _wrapper_view return view_func(request, *args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "integreat-cms/.venv/lib/python3.11/site-packages/django/views/generic/base.py", line 143, in dispatch return handler(request, *args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "integreat-cms/integreat_cms/cms/utils/tree_mutex.py", line 117, in innermost_function return func(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^ File "integreat-cms/integreat_cms/cms/views/pages/page_form_view.py", line 305, in post if not page_form.is_valid() or not page_translation_form.is_valid(): ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "integreat-cms/.venv/lib/python3.11/site-packages/django/forms/forms.py", line 201, in is_valid return self.is_bound and not self.errors ^^^^^^^^^^^ File "integreat-cms/.venv/lib/python3.11/site-packages/django/forms/forms.py", line 196, in errors self.full_clean() File "integreat-cms/.venv/lib/python3.11/site-packages/django/forms/forms.py", line 433, in full_clean self._clean_fields() File "integreat-cms/.venv/lib/python3.11/site-packages/django/forms/forms.py", line 448, in _clean_fields value = getattr(self, "clean_%s" % name)() ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "integreat-cms/integreat_cms/cms/forms/custom_content_model_form.py", line 178, in clean_content "Image tag found in content (src: %s)", image.attrib["src"] ~~~~~~~~~~~~^^^^^^^ File "src/lxml/etree.pyx", line 2502, in lxml.etree._Attrib.__getitem__ KeyError: 'src' Apr 02 16:03:53 ERROR django.server - "POST /augsburg/pages/de/1/edit/ HTTP/1.1" 500 20360 ```
MizukiTemma commented 3 months ago

@PeterNerlich I'm thinking what kind of hint message should be shown to users.

Many of them probably don't know what an tag is and how it should look like. Then it probably doesn't help to say "The text contains a broken tag. Please check it" or somehting like that if they don't know how to fix it 😓

But the content cannot be saved as long as there is an broken img tag 🤔 Should we remove broken tags, save the content and tell users, for example, "Did you try to insert an image? Something went wrong. The page/location/event was saved without it. Please try again."

PeterNerlich commented 3 months ago

it probably doesn't help to say "The text contains a broken tag. Please check it" or somehting like that if they don't know how to fix it 😓

It is not anything else than images, right? And an image without a source should be allowed by the HTML spec, AFAIK. Can we just skip the image tag for media checking if the source is missing to prevent the issue and display a message that an image might be broken?