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

Check the input and destination paths before converting a game file onto itself #11576

Merged

Conversation

yannhodiesne
Copy link
Contributor

Before these changes you could tell Dolphin to convert a game file into the same format it is already in, leading to the FileDialog using the input path as the default destination path

An unsuspecting user could then click Save and Dolphin would try to convert the input file by writing the destination file on top of it... leading to an I/O error and the input file being entirely removed

Instead of denying the user the ability to select the format already used by the input file (which would be annoying if someone wants to convert from RVZ to RVZ but with other compression settings), we can check if the input and destination paths are equal and error out before having the input file accidentally removed

@AdmiralCurtiss
Copy link
Contributor

AdmiralCurtiss commented Feb 17, 2023

Good idea, but now that std::filesystem works you might actually want to do this with something like

#include <filesystem>
#include "Common/StringUtil.h"


        std::error_code ec;
        if (std::filesystem::equivalent(StringToPath(dst_path.toStdString()),
                                        StringToPath(original_path), ec))
        {
          ModalMessageBox::critical(
              this, tr("Error"),
              tr("The destination file cannot be the same as the source file\n\n"
                 "Please select another destination path for \"%1\"")
                  .arg(QString::fromStdString(original_path)));
          continue;
        }

(after checking if dst_path exists)

That also catches stuff like symlinks where the path is different but the underlying file is the same.

@yannhodiesne
Copy link
Contributor Author

Thanks for the advice! I'm gonna do it your way, I just want to add a counter of successfully converted files to avoid having a "Successfully converted 1 image(s)." right after the error message, but I don't know if I should do it in another PR or not
I'll do it in two separate commits to make it easier to split later

…nto itself

Before these changes you could tell Dolphin to convert a game file into the same format it is already in, leading to the FileDialog using the input path as the default destination path
An unsuspecting user could then click Save and Dolphin would try to convert the input file by writing the destination file on top of it... leading to an I/O error and the input file being entirely removed
@AdmiralCurtiss
Copy link
Contributor

Oh what, Qt doesn't allow unsigned values as input to %n? That's silly... I guess revert to int then, whatever.

@yannhodiesne
Copy link
Contributor Author

Oh what, Qt doesn't allow unsigned values as input to %n? That's silly... I guess revert to int then, whatever.

Shoot, didn't catch it before pushing, it worked on macOS

@AdmiralCurtiss
Copy link
Contributor

m_files.size() is also size_t and that didn't generate a warning, so I'm actually a bit confused here... but whatever, it's not like anyone will convert over 2 billion ISOs at the same time.

@JosJuice
Copy link
Member

m_files is a QList, so its size function does actually return int.

@AdmiralCurtiss AdmiralCurtiss merged commit 74abf48 into dolphin-emu:master Feb 17, 2023
@yannhodiesne yannhodiesne deleted the convert-prevent-file-deletion branch February 17, 2023 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants