SQL panel: data of query (stored in a HTML hidden field) leaks into action column

[karolyi]

Hey,

I'm using the latest toolbar, and I noticed that somehow the 'action' column of the SQL panel sometimes contains some SQL-looking code, that pushes every other columns to the smallest available width, this using the panel unreadable. A screenshot:

Upon looking into it with devtools (I'm using Librewolf), it seems the leaking text should be stored in the value of a html hidden input field, but somehow only part of it does:

Right now I have no idea as to what's happening here. I'm using a pretty default toolbar config, with nothing changed.

I'm open to questions to figure out what this is.

[karolyi]

The HTML source shows this, it seems the quote isn't escaped and it breaks the value field:

[tim-schilling]

Thanks for the report @karolyi! If you have time to identify the fix or open a PR, that would go a long way to getting a fix out quickly.

[karolyi]

Hey,

unfortunately I don't have time to make myself acquainted with the inner workings of django-debug-toolbar, but I'm happy to provide more information upon request.

[matthiask]

I'm unable to reproduce this. Can you provide additional details, e.g. the database engine and the versions of involved packages you're using? (Django, sqlparse etc.)?

I'm surprised that this happens. We're not doing anything special with the signed data, we're putting it into a Django HiddenInput without marking it as safe (mark_safe and friends) and let Django do the rest.

[karolyi]

Hey,

of course.

The database engine is MariaDB, with using mysqlclient==2.1.1, sqlparse==0.4.3, Django==4.1.8, and django-debug-toolbar==4.0.0.

If there's anything else you need, let me know.

[karolyi]

My main templating engine is Jinja:

TEMPLATES = [
    {
        'BACKEND': 'django_jinja.backend.Jinja2',
        'APP_DIRS': True,
        'DIRS': [],
        'OPTIONS': {
            'app_dirname': 'jinja2',
            'match_extension': None,
            # 'match_regex': r'^(?!debug_toolbar/).*',
            'newstyle_gettext': True,
            # https://github.com/niwinz/django-jinja/issues/140#issuecomment-145696119
            'filters': {
                'add_class':
                    'widget_tweaks.templatetags.widget_tweaks.add_class',
                'set_attr':
                    'widget_tweaks.templatetags.widget_tweaks.set_attr',
            },
            'extensions': DEFAULT_EXTENSIONS + [
                'django_jinja.builtins.extensions.CsrfExtension',
                'django_jinja.builtins.extensions.DjangoExtraFiltersExtension',
                'webpack_loader.contrib.jinja2ext.WebpackExtension',
                'waffle.jinja.WaffleExtension',
                f'{PROJECT_NAMESPACE}.jinja2.SpacelessExtension',
                f'{PROJECT_NAMESPACE}.jinja2.' +
                f'{PROJECT_NAMESPACE_CAMELCASE}ToolsExtension',
                f'{PROJECT_NAMESPACE}.jinja2.JsMinExtension',
                f'{PROJECT_NAMESPACE}.jinja2.CommentExtension',
                f'{PROJECT_NAMESPACE}.jinja2.MyLanguageInfoExtension',
            ],
            'bytecode_cache': {
                'name': 'default',
                'backend': 'django_jinja.cache.BytecodeCache',
                'enabled': not DEBUG,
            },
            'autoescape': False,
            'auto_reload': DEBUG,
            'translation_engine': 'django.utils.translation',
        },
    },
    {
        'BACKEND': 'django.template.backends.django.DjangoTemplates',
        'DIRS': [],
        'APP_DIRS': True,
        'OPTIONS': {
            'context_processors': [
                'django.contrib.auth.context_processors.auth',
                'django.contrib.messages.context_processors.messages',
                'django.template.context_processors.debug',
                'django.template.context_processors.i18n',
                'django.template.context_processors.media',
                'django.template.context_processors.request',
                'django.template.context_processors.static',
                'django.template.context_processors.tz',
            ],
        },
    },
]

[matthiask]

Thanks!

Thinking out loud: Maybe we should use signing.dumps and signing.loads, or Signing.sign_object since this method also base64 encodes and optionally compresses the data. This would mean that the value in the hidden input is only readable when using tools but that should be fine, really.

[matthiask]

... urlsafe base64 encoding completely avoids the problems around escaping, that's why I'm suggesting this.

[karolyi]

If my opinion counts, I support whatever works. :)

[tim-schilling]

@karolyi This is likely related to django-jinja rendering a hidden input with JSON inside of it. Can you share the relevant information to reproduce your project? Your django-jinja set up refers to a number of non-default extensions. I'm not familiar with jinja too much so we'll need your help recreating this environment.

Alternatively, you could create a small project that's a minimal example that reproduces the effect.

[karolyi]

Hey,

at your service. :)

I've prepared a small test repo where you can reproduce the bug: https://gitea.ksol.io/karolyi/ddt-1756

Do a requirements install, and then migrate, and then a runserver. Then point your browser to /test/, reload once more (the first iteration will put a variable into the session DB the later requests use), and on second reload you'll be able to see the JSON data in the hidden field reachable with the CSS selector .djdt-actions form[method=post] input[name=signed].

Hope I could help.

[tim-schilling]

@karolyi I think this may be a browser issue. I can't reproduce this.

When I inspect the rendered HTML, it does appear to look like what you show, but when I go to copy it in the browser dev tools, it shows that it's properly HTML escaped:

{"vendor": "sqlite", "alias": "default", "sql": "SELECT \"django_session\".\"session_key\", \"django_session\".\"session_data\", \"django_session\".\"expire_date\" FROM \"django_session\" WHERE (\"django_session\".\"expire_date\" > '2023-10-23 01:12:38.091261' AND \"django_session\".\"session_key\" = 'i0bw0vly2w12mp356fh042s6gyras4sg') LIMIT 21&q
...

I'm going to close this as won't fix unless you can identify where in our code it's breaking.

[karolyi]

I'm gonna give this test another go tomorrow, but my hunch is the 'autoescape': False setting.