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

Error message sometimes missing when inter-qube file copy/move fails due to disallowed characters in filename #8738

Closed
xqb34 opened this issue Dec 1, 2023 · 42 comments · Fixed by QubesOS/qubes-core-agent-linux#476 or QubesOS/qubes-linux-utils#108
Labels
affects-4.2 This issue affects Qubes OS 4.2. C: core diagnosed Technical diagnosis has been performed (see issue comments). P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. pr submitted A pull request has been submitted for this issue. r4.2-host-stable r4.2-vm-bookworm-stable r4.2-vm-bullseye-stable r4.2-vm-centos-stream8-stable r4.2-vm-fc37-stable r4.2-vm-fc38-stable r4.2-vm-fc39-stable T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists.
Milestone

Comments

@xqb34
Copy link

xqb34 commented Dec 1, 2023

Qubes OS release

4.2.0
Installed RC4 and installed updates since then.

Brief summary

As discussed in #8332 , copy/move files can fail when there are non-english characters, and that is an intended behavior.

However, copy/move sometimes fails without any warning message, which can result in loss of data.

Steps to reproduce

Create a file named عربية or עברית, e.g. touch عربية or touch עברית.
From the file manager, choose copy to other appVM.

Expected behavior

File is copied or an error message is presented.

Actual behavior

File not copied and no error message is displayed to user.

I haven't checked if command line provides any error indication, but if it does not, it would create issues with automatic scripts.

As side note, I am sure that the choice to have strict file name validation has been debated. I personally find that it damages usability (even if there is an error message) because user have existing files and receive files from others. I am sure that there are good security reasons for the choice.

@xqb34 xqb34 added P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists. labels Dec 1, 2023
@xqb34 xqb34 changed the title Copy/Move with non english characters fails withtout warning message. Copy/Move with non english characters fails withtout a warning message. Dec 1, 2023
@jamke

This comment was marked as off-topic.

@xqb34

This comment was marked as off-topic.

@marmarek

This comment was marked as off-topic.

@jamke

This comment was marked as off-topic.

@DemiMarie

This comment was marked as off-topic.

@jamke

This comment was marked as off-topic.

@jamke

This comment was marked as off-topic.

@marmarek

This comment was marked as off-topic.

@marmarek

This comment was marked as off-topic.

@xqb34

This comment was marked as off-topic.

@jamke

This comment was marked as off-topic.

@jamke

This comment was marked as off-topic.

@jamke

This comment was marked as off-topic.

@rustybird
Copy link

rustybird commented Dec 1, 2023

Could we keep this ticket focused though...

Create a file named عربية or עברית, e.g. touch عربية or touch עברית.

Just so we're all on the same page, in a Bash shell that should be identical to touch $'\330\271\330\261\330\250\331\212\330\251' and touch $'\327\242\327\221\327\250\327\231\327\252' respectively?

(1) What's the sticking point with these strings?

I've tried a few online Unicode analyzers such as https://unicodedecode.com/ and they both seem to be valid UTF-8, made of ordinary(?) Arabic or Hebrew letters, without directionality changes or embedded right-to-left markers.

From the file manager, choose copy to other appVM.

File not copied and no error message is displayed to user.

(2) The filecopy GUI should surface any error to the user.

@marmarek
Copy link
Member

marmarek commented Dec 1, 2023

(1) What's the sticking point with these strings?

Implicit right-to-left: https://github.com/QubesOS/qubes-linux-utils/blob/main/qrexec-lib/unicode-generator.c#L180-L190

@rustybird
Copy link

Harsh (and a point in favor of adding less restrictive modes), but not unreasonable as a default IMO.

Leaving (2), why is qfile-agent screaming its error message into a void/terminal instead of zenity for PROGRESS_TYPE=gui.

@DemiMarie

This comment was marked as off-topic.

@andrewdavidwong andrewdavidwong changed the title Copy/Move with non english characters fails withtout a warning message. Missing error message when qvm-copy/qvm-move fails due to disallowed characters in filename Dec 2, 2023
@andrewdavidwong andrewdavidwong added affects-4.2 This issue affects Qubes OS 4.2. C: core needs diagnosis Requires technical diagnosis from developer. Replace with "diagnosed" or remove if otherwise closed. labels Dec 2, 2023
@andrewdavidwong
Copy link
Member

andrewdavidwong commented Dec 2, 2023

If I understand correctly, this issue is only about there sometimes not being an error message when inter-qube file copy/move fails due to disallowed characters in the filename. It is not about whether the failure itself is desirable or should be changed (that's #8332). The point here is simply that when the failure occurs, there should always be a suitable error message, which is not currently the case.

@andrewdavidwong andrewdavidwong changed the title Missing error message when qvm-copy/qvm-move fails due to disallowed characters in filename Error message sometimes missing when qvm-copy/qvm-move fails due to disallowed characters in filename Dec 2, 2023
@andrewdavidwong andrewdavidwong changed the title Error message sometimes missing when qvm-copy/qvm-move fails due to disallowed characters in filename Error message sometimes missing when inter-qube copy/move fails due to disallowed characters in filename Dec 2, 2023
@andrewdavidwong andrewdavidwong changed the title Error message sometimes missing when inter-qube copy/move fails due to disallowed characters in filename Error message sometimes missing when inter-qube file copy/move fails due to disallowed characters in filename Dec 2, 2023
rustybird added a commit to rustybird/qubes-core-agent-linux that referenced this issue Dec 2, 2023
In practice kdialog wasn't used, because the package was built without
USE_KDIALOG being defined.

Fixes QubesOS/qubes-issues#8738
@qubesos-bot
Copy link

Automated announcement from builder-github

The package linux-utils has been pushed to the r4.2 stable repository for the CentOS centos-stream8 template.
To install this update, please use the standard update command:

sudo yum update

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package linux-utils has been pushed to the r4.2 stable repository for the Debian template.
To install this update, please use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package linux-utils has been pushed to the r4.2 stable repository for the Debian template.
To install this update, please use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The component linux-utils (including package linux-utils) has been pushed to the r4.2 stable repository for the Fedora template.
To install this update, please use the standard update command:

sudo dnf update

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The component linux-utils (including package linux-utils) has been pushed to the r4.2 stable repository for the Fedora template.
To install this update, please use the standard update command:

sudo dnf update

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The component linux-utils (including package linux-utils) has been pushed to the r4.2 stable repository for the Fedora template.
To install this update, please use the standard update command:

sudo dnf update

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package core-agent-linux has been pushed to the r4.2 stable repository for the CentOS centos-stream8 template.
To install this update, please use the standard update command:

sudo yum update

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package core-agent-linux has been pushed to the r4.2 stable repository for the Debian template.
To install this update, please use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

1 similar comment
@qubesos-bot
Copy link

Automated announcement from builder-github

The package core-agent-linux has been pushed to the r4.2 stable repository for the Debian template.
To install this update, please use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The component core-agent-linux (including package core-agent-linux) has been pushed to the r4.2 stable repository for the Fedora template.
To install this update, please use the standard update command:

sudo dnf update

Changes included in this update

2 similar comments
@qubesos-bot
Copy link

Automated announcement from builder-github

The component core-agent-linux (including package core-agent-linux) has been pushed to the r4.2 stable repository for the Fedora template.
To install this update, please use the standard update command:

sudo dnf update

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The component core-agent-linux (including package core-agent-linux) has been pushed to the r4.2 stable repository for the Fedora template.
To install this update, please use the standard update command:

sudo dnf update

Changes included in this update

marmarek pushed a commit to QubesOS/qubes-linux-utils that referenced this issue Jan 27, 2024
For PROGRESS_TYPE=gui the error handler calls zenity, which fails to
launch if the error message contains characters not supported by the
locale's charset (e.g. LC_CTYPE=C).

Filenames in the error message were already sanitized to printable
ASCII, but then enclosed in UTF-8 curly quotation marks.

Fixes QubesOS/qubes-issues#8738

(cherry picked from commit 71f5b04)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-4.2 This issue affects Qubes OS 4.2. C: core diagnosed Technical diagnosis has been performed (see issue comments). P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. pr submitted A pull request has been submitted for this issue. r4.2-host-stable r4.2-vm-bookworm-stable r4.2-vm-bullseye-stable r4.2-vm-centos-stream8-stable r4.2-vm-fc37-stable r4.2-vm-fc38-stable r4.2-vm-fc39-stable T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists.
Projects
None yet
7 participants