Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTML parsing in attributes #5755

Closed
mdhooge opened this issue Nov 27, 2020 · 4 comments · Fixed by #5834
Closed

HTML parsing in attributes #5755

mdhooge opened this issue Nov 27, 2020 · 4 comments · Fixed by #5834
Milestone

Comments

@mdhooge
Copy link
Contributor

mdhooge commented Nov 27, 2020

Overview

When attributes are printed on the "Advanced" tab in the "Preview" area, the text is analysed for HTML tags.
Whenever you have a single "inferior" sign, the sign and everything after is not printed.

Steps to Reproduce

  1. Create an attribute with text “1<2”
  2. Create another attribute with text “1& lt;2” (Remove the space after the ampersand — it is there to prevent Github Markdown to trigger a conversion…)

Expected Behavior

For #1, we should see the text “1<2“.
For #2, I'm not sure…

I don't know if this HTML parsing is done on purpose but this is misleading. If you have a long password with an inferior sign at the end, you can easily overlook the fact that some characters are missing!

Actual Behavior

1. “1”
2. “1<2“

Context

KeePassXC - Version 2.6.2
Revision: e9b9582

Qt 5.15.1
Debugging mode is disabled.

Operating system: Arch Linux
CPU architecture: x86_64
Kernel: linux 5.9.8-arch1-1

Enabled extensions:

  • Auto-Type
  • Browser Integration
  • SSH Agent
  • KeeShare (signed and unsigned sharing)
  • YubiKey
  • Secret Service Integration

Cryptographic libraries:
libgcrypt 1.8.7

Operating System: ArchLinux
Desktop Env: KDE
Windowing System: X11

@mdhooge mdhooge added the bug label Nov 27, 2020
@droidmonkey
Copy link
Member

Thats definitely a mistake if enabled

@mdhooge
Copy link
Contributor Author

mdhooge commented Nov 28, 2020

In fact, this is enabled to easily format the output!

In file src/gui/EntryPreviewWidget.cpp, method updateEntryAdvancedTab, replace the for loop by something like below (Note: I didn't try to compile it — but it is really dumb!).

        for (const QString& key : customAttributes) {
            QString value;
            if (m_currentEntry->attributes()->isProtected(key)) {
                value = "<i>" + tr("[PROTECTED]") + "</i>";
            } else {
                value = m_currentEntry->attributes()->value(key).toHtmlEscaped();
            }
            attributesText.append(tr("<b>%1</b>: %2", "attributes line").arg(key, value).append("<br/>"));
        }

Ref: https://doc.qt.io/qt-5/qstring.html#toHtmlEscaped

@mdhooge
Copy link
Contributor Author

mdhooge commented Nov 29, 2020

Nice to have additions:

  • If content has LineFeeds, replace them by < br / >
  • If Settings are to use Monospace font for Notes, also enforce it here.

@droidmonkey droidmonkey added this to the v2.7.0 milestone Dec 12, 2020
@mdhooge
Copy link
Contributor Author

mdhooge commented Dec 15, 2020

* If content has LineFeeds, replace them by < br / >

Done in #5834

* If Settings are to use Monospace font for Notes, also enforce it here.

Maybe I should open a new issue?

droidmonkey pushed a commit to mdhooge/keepassxc that referenced this issue Dec 19, 2020
* Fix keepassxreboot#5755 - HTML escape attributes prior to preview
* Place attribute preview into a table and convert line breaks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants