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

Carriage returns in file names cause corrupted verbose output #4191

Closed
quinncomendant opened this issue Feb 5, 2023 · 5 comments
Closed

Comments

@quinncomendant
Copy link

Output of restic version

restic 0.15.0 compiled with go1.19.5 on darwin/arm64

How did you run restic exactly?

export RESTIC_PASSWORD_COMMAND='security find-generic-password -a (redacted) -s (redacted) -w';
export GOOGLE_PROJECT_ID='(redacted)';
export GOOGLE_APPLICATION_CREDENTIALS="(redacted)";
export RESTIC_REPOSITORY='gs:(redacted):/';

caffeinate -i restic -v -v backup "$HOME"

What backend/server/service did you use to store the repository?

GCP Coldline storage

Expected behavior

If a filename contains a carriage return, it should be sanitized before printing to the terminal.

Actual behavior

When backing up file names containing carriage returns, the verbose output gets corrupted because the carriage returns are output directly to the terminal, sending the following output to the beginning of the lines. Here's a a real example. Dunno why, but I have a folder whos name contains a carriage return character: Linux Admin Security Guide<CR>. The verbose output prints like:

/servers/, saved in 0.041s (0 B added, 0 B stored, 0 B metadata)in Security Guide
/software/, saved in 0.002s (0 B added, 0 B stored, 0 B metadata)n Security Guide
/system/, saved in 0.002s (0 B added, 0 B stored, 0 B metadata)min Security Guide
/users/, saved in 0.002s (0 B added, 0 B stored, 0 B metadata)dmin Security Guide
/viruses/, saved in 0.002s (0 B added, 0 B stored, 0 B metadata)in Security Guide
/vpn/, saved in 0.001s (0 B added, 0 B stored, 0 B metadata) Admin Security Guide
/, saved in 0.120s (0 B added, 0 B stored, 0 B metadata)inux Admin Security Guide
modified  /Users/q/Bibliotheque/technology/Unix/_books/Linux Admin survival guide/, saved in 0.020s (0 B added, 0 B stored, 0 B metadata)
modified  /Users/q/Bibliotheque/technology/Unix/_books/Linux From Scratch/, saved in 0.118s (0 B added, 0 B stored, 0 B metadata)

These first seven lines should appear more like:

modified  /Users/q/Bibliotheque/technology/Unix/_books/Linux Admin Security Guide/servers/, saved in 0.1s (0 B added, 0 B stored, 0 B metadata)
…

Steps to reproduce the behavior

  1. Add a carriage return to a filename
  2. Run restic backup with verbose output enabled.

Do you have any idea what may have caused this?

Special characters in file names are printed as-is to the terminal.

Do you have an idea how to solve the issue?

Special characters in file names should be sanitized before printing to the terminal.

Did restic help you today? Did it make you happy in any way?

Restic is awesome. Thanks!

@rawtaz
Copy link
Contributor

rawtaz commented Feb 5, 2023

Is it not better to fix the actual shenanigans in your filenames instead of shoving them under the carpet? If restic silently sanitizes CR you will keep having bad filenames ;)

@greatroar
Copy link
Contributor

You can't expect users to change the data they're going to back up to suit the backup program. That goes against the idea of backups.

greatroar added a commit to greatroar/restic that referenced this issue Feb 5, 2023
greatroar added a commit to greatroar/restic that referenced this issue Feb 5, 2023
@rawtaz
Copy link
Contributor

rawtaz commented Feb 5, 2023

You can't expect users to change the data they're going to back up to suit the backup program.

Thanks, but you seem to have completely misread what I wrote. I never suggested that users change their data to suit the backup program.

I asked them if they wouldn't think that fixing the actual root cause of the problem is a good idea, instead of just hiding the problem. Simply because fixing things instead of hiding things is generally a good idea.

That goes against the idea of backups.

The idea of backups is to back data up, and this is exactly what restic does regardless of whether there's a CR in the filename or not.


I also never suggested that we should not clean broken filenames up when printing them.

@greatroar
Copy link
Contributor

Alright, misunderstanding on my part. Sorry!

greatroar added a commit to greatroar/restic that referenced this issue Feb 10, 2023
greatroar added a commit to greatroar/restic that referenced this issue Feb 10, 2023
greatroar added a commit to greatroar/restic that referenced this issue Feb 11, 2023
greatroar added a commit to greatroar/restic that referenced this issue Feb 11, 2023
greatroar added a commit to greatroar/restic that referenced this issue Feb 11, 2023
greatroar added a commit to greatroar/restic that referenced this issue Feb 11, 2023
greatroar added a commit to greatroar/restic that referenced this issue Feb 11, 2023
greatroar added a commit to greatroar/restic that referenced this issue Apr 12, 2023
greatroar added a commit to greatroar/restic that referenced this issue Apr 14, 2023
MichaelEischer pushed a commit to MichaelEischer/restic that referenced this issue Apr 23, 2023
@MichaelEischer
Copy link
Member

Fixed by #4192

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants