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

Fix freeze when opening repo on unsupported URL scheme. #1631

Open
real-yfprojects opened this issue Mar 8, 2023 · 72 comments
Open

Fix freeze when opening repo on unsupported URL scheme. #1631

real-yfprojects opened this issue Mar 8, 2023 · 72 comments
Labels
good first issue Simple change to start learning code base help wanted This issue is available, comment if you want to fix it type:bug Something doesn't work as intended

Comments

@real-yfprojects
Copy link
Collaborator

real-yfprojects commented Mar 8, 2023

It seems like not all file explorers respect the supported scheme property, that's why Vorta shouldn't rely on that. However currently Vorta will freeze when opening an unsupported scheme like smb or sftp. This should be fixed. A small error message should be displayed in the dialog instead.


You can open smb:// and other URLs when adding a repository although borg only supports normal paths (file scheme) and the ssh scheme. This is because vorta is missing to set the supported schemes attribute.

See https://doc.qt.io/qt-5/qfiledialog.html#supportedSchemes-prop

Originally posted by @ThomasWaldmann in #1628 (comment)

@real-yfprojects real-yfprojects added type:bug Something doesn't work as intended good first issue Simple change to start learning code base help wanted This issue is available, comment if you want to fix it labels Mar 8, 2023
@i1sm3ky
Copy link
Contributor

i1sm3ky commented Mar 9, 2023

@real-yfprojects @ThomasWaldmann Would dialog.setSupportedSchemes(["file", "ssh"]) work?

@real-yfprojects
Copy link
Collaborator Author

I think so.

@ThomasWaldmann
Copy link
Collaborator

Not sure whether the file dialog will be able to let you "select" a ssh: url somehow, but we can try.

What it for sure won't do, is the special ssh: path styles borg uses to indicate current directory and home directory on the server, usually ssh:// urls always give absolute paths only.

@real-yfprojects
Copy link
Collaborator Author

Not sure whether the file dialog will be able to let you "select" a ssh: url somehow, but we can try.

I was thinking that there might be a file selector somewhere out there that allows just that, so why not tell it that vorta supports ssh://.

What it for sure won't do, is the special ssh: path styles borg uses to indicate current directory and home directory on the server, usually ssh:// urls always give absolute paths only.

Are normal ssh: paths supported by borg?

@ThomasWaldmann
Copy link
Collaborator

ThomasWaldmann commented Mar 9, 2023

ssh://user@host:port/some/abs/path is the usual and supported form.

ssh://user@host:port/./some/rel/path and ssh://user@host:port/~/some/home/path is supported by borg, but likely nowhere else.

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Mar 9, 2023

So should we just allow the local path to be added using the dialog box and leave the ssh to be filled manually?

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Mar 10, 2023

@real-yfprojects so I just add dialog.setSupportedSchemes(["file", "ssh"]) and merge it right?

@ThomasWaldmann
Copy link
Collaborator

@i1sm3ky i guess this needs trying interactively. try it before any change, check how the dialogue behaves and what it offers (esp. considering non-file: stuff), then change the schemes and try again how it behaves.

this is platform dependent, so try on all platforms you have access to.

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Mar 10, 2023

@i1sm3ky i guess this needs trying interactively. try it before any change, check how the dialogue behaves and what it offers (esp. considering non-file: stuff), then change the schemes and try again how it behaves.

this is platform dependent, so try on all platforms you have access to.

Ok, I'll test that and work on it accordingly.

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Mar 11, 2023

I tested it for local repos on MacOS Ventura 13.2.1 and it worked without any errors for both the cases(with and without the schemes). Unfortunately I can't figure out a way to test it for ssh and smb. Is there a way to test it for smb and ssh(without a paid host like borgbase) on the same machine?

Also while installing Borg, I faced some issues as I'm using a m1 chip and wasn't able to install the arm version. I had to install it using Rosetta 2 on x86_64 architecture. I think an update on https://vorta.borgbase.com/install/macos/ instructing on how to install it for m1 would be good.

@ThomasWaldmann
Copy link
Collaborator

There is nothing you need to "test" for smb except that:

  • before your modification you can select "smb:" stuff somehow
  • after your modification it will not let you choose "smb:" stuff any more

@ThomasWaldmann
Copy link
Collaborator

ThomasWaldmann commented Mar 11, 2023

For ssh, you could run a sshd on localhost or some other (e.g. linux) machine you have access to.

borgbase has a for-free offer btw.

@ThomasWaldmann

This comment was marked as off-topic.

@real-yfprojects
Copy link
Collaborator Author

You should be able to use a VM or a different machine to set up a test smb share and test ssh access.

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Mar 11, 2023

before your modification you can select "smb:" stuff somehow
after your modification it will not let you choose "smb:" stuff any more

Ya I kinda was asking for how can I do that as I didn't have another machine with me at that point of time. But I'll use one of my friends to test.

For ssh, you could run a sshd on localhost or some other (e.g. linux) machine you have access to.
borgbase has a for-free offer btw.

Ok I'll look more into that. Thanks ;)

borg runs fine (and natively, without rosetta) on apple silicon, I have a MBA M1 myself.

Ok... Maybe there was something wrong on my end.

You should be able to use a VM or a different machine to set up a test smb share and test ssh access.

I tried that with a VM but was unable to do so, I'll try it once again.

@ThomasWaldmann
Copy link
Collaborator

ThomasWaldmann commented Mar 11, 2023

I just tried on my MBA and I was not able to get a "smb:" URL result from the file dialogue (when adding a new repo):

  • either I click on the "world" icon, then it just gives me a ssh: prototype url to edit (no file dialogue)
  • or i click on the "folder" icon, then I can select misc simple paths from the filesystem (not: URLs) using the file dialogue
  • even when trying to select a SMB share via the file dialogue, the SMB server needs to be "connected" first and that will result in the share being mounted below `/Volumes/..." and it will also just result in a normal fs path (not an URL).
  • i was not able to select something from the SMB server without "connecting" (mounting) it first.

I could not discover any way to select a ssh: URL via the file dialogue.

So, as far as macOS and smb: / ssh: is concerned, I did not find the problem we are trying to "solve" here.

vorta 0.8.10 borg 1.2.3 macOS Ventura

@real-yfprojects
Copy link
Collaborator Author

real-yfprojects commented Mar 11, 2023

I did not find the problem we are trying to "solve" here.

The guy in #1628 used Gnome and Fedora 37.

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Mar 11, 2023

i was not able to select something from the SMB server without "connecting" (mounting) it first.

Yes same, I was able to use a mounted server but was not able to select a smb:// or a URL by itself on mac.

The guy in #1628 used Gnome and Fedora 37.

Yeah I'll try on those to see if I can reproduce the issue and does the fix work.

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Mar 19, 2023

Sorry, I had my exams so I was unable to work on the issue.

I got one of my friend's pc to test.

While testing, I found some more issues-

  • When selecting ssh from the file dialog it also froze vorta
  • Manually typing the ssh or smb in the text field, both gave error: Unable to add your repository

Unfortunately I was not able to check with the fix enabled as I'm having some issues with linux. I'll try to resolve the issue and test the fix tomorrow.

I think it may also be a issue with Flatpak. It uses Sandboxing to increase security, On their site they have mentioned - by default, applications that are run with Flatpak have extremely limited access to the host environment. This includes: no access to the network.

And I also would love to know what would be the chances of getting GSOC under your mentorship. I am really looking forward to work more with you guys. What should be my next step and how should I apply to GSOC for your organisation?
@real-yfprojects @m3nu @ThomasWaldmann Any help or recommendations would really be appreciated. :)

Edit: And when should I apply?

@real-yfprojects
Copy link
Collaborator Author

And I also would love to know what would be the chances of getting GSOC under your mentorship. I am really looking forward to work more with you guys. What should be my next step and how should I apply to GSOC for your organisation?

Chances are good. You can find information about our application process on our GSoC wiki page. You have already completed steps one to three.

  • When selecting ssh from the file dialog it also froze vorta

What file manager / dialog allows you to select ssh urls?

  • Manually typing the ssh or smb in the text field, both gave error: Unable to add your repository

A valid ssh address to a borg repository should work. The behaviour regarding smb is expected.

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Mar 20, 2023

Chances are good. You can find information about our application process on our GSoC wiki page. You have already completed steps one to three.

Yeah Thanks :) I have read that earlier, I'll again have look into that and apply shortly.
One more thing, if I send you the application before uploading to google, could any of you please check it once and tell me the corrections if any is need?

What file manager / dialog allows you to select ssh urls?

The default Fedora one.

A valid ssh address to a borg repository should work.

It didn't in my case.

@real-yfprojects

This comment was marked as off-topic.

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Mar 20, 2023

Yes, that is the recommended procedure. This is also laid out on the linked page.

Yup, Thanks again ;)

I think regarding the file dialog, We should only allow local files to be selected and ssh and other supported URLs could be added using the text box itself. So only allowing local files to be selected using the file dialog by adding dialog.setSupportedSchemes(["file", ]) should get us the expected functionality we are looking for. I'll just check it once and confirm in about half an hour or so.

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Mar 20, 2023

Setting dialog.setSupportedSchemes(["file", ]) didn't helped either. I was still able to select ssh and smb URLs from the file dialog.
I tried on Ubuntu as well, it took the ssh URL as sftp URLs and didn't prevent me from selecting a file over a network. And on Ubuntu, I didn't really found a way to select smb file from the file dialog by default, the file dialog didn't had the connect to a network tab on there.

Edit- Another solution would be to check for substrings containing unsupported URL types in the selected URL and pushing a QMessageBox informing the user about the supported URL types.

@ThomasWaldmann
Copy link
Collaborator

If the file dialogues are misbehaved and do not respect parameters given:

  • a bug should be filed at the respective projects
  • we could just not use the file dialogue - typing in a fs path or a ssh:// URL isn't that hard and preferable to a file dialogue crashing/hanging vorta just because the user clicked on something unexpected.

@real-yfprojects
Copy link
Collaborator Author

  • we could just not use the file dialogue - typing in a fs path or a ssh:// URL isn't that hard and preferable to a file dialogue crashing/hanging vorta just because the user clicked on something unexpected.

This would make for a very bad UX though. Vorta shouldn't crash/freeze instead.

@m3nu
Copy link
Contributor

m3nu commented Mar 28, 2023

Yes, just link to the issue.

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Mar 28, 2023

Yup sure, Thanks!

@ThomasWaldmann
Copy link
Collaborator

@i1sm3ky if you decide to write a own url/path validator, please do it as a boolean function and write unit tests for it.

but as i said, it might be simpler to either copy that from borg code (or invoke a borg command to let it do the testing).

if you write your own REs and conditions, it is rather likely that you miss(match) something.

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Mar 28, 2023

@ThomasWaldmann Thanks for the guide, I'll check the borg code for the url validator and the unit tests. Sorry for the inconvenience, I just looked past the fact that borg also does a url check, I don't know what was I thinking of :/
I'll see the borg code and implement it and share it with you soon. Could you please help me locate the files which may contain the url validator and the unit tests for borg as I haven't been much involved in the borg code base.

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Mar 28, 2023

Found the REs and the location_validator on borg.

@real-yfprojects
Copy link
Collaborator Author

Edit: In the PSF checklist they have mentioned Your code doesn't have to be accepted and merged, but it does have to be visible to the public and it does have to be your own work so a link to the issues where the code have been discussed should work right?

Opening a PR would be even better!

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Mar 30, 2023

Opening a PR would be even better!

Ya that's why I'm trying to get it done before 4th so that I can include it as a PR in the proposal.

@real-yfprojects
Copy link
Collaborator Author

You can also open a draft PR so we can better see and comment on your current progress.

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Mar 30, 2023

Yeah sure! I'll do that.

@real-yfprojects
Copy link
Collaborator Author

Why does vorta freeze in the first place?

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Apr 1, 2023

Sorry for the delayed reply.
I think its because selected file URL type isn't supported and doesn't get processed at some point and Vorta doesn't get a result out of it so it freezes waiting for a result to work upon.

@real-yfprojects
Copy link
Collaborator Author

Sorry for the delayed reply.

Your reply isn't late at all.

I think its because selected file URL type isn't supported and doesn't get processed at some point and Vorta doesn't get a result out of it so it freezes waiting for a result to work upon.

Can you triage which lines of code cause the freeze?

@i1sm3ky

This comment was marked as off-topic.

@ThomasWaldmann

This comment was marked as off-topic.

@i1sm3ky

This comment was marked as off-topic.

@ThomasWaldmann

This comment was marked as off-topic.

@i1sm3ky

This comment was marked as off-topic.

@real-yfprojects

This comment was marked as off-topic.

@i1sm3ky

This comment was marked as off-topic.

@TheLazron

This comment was marked as off-topic.

@ThomasWaldmann

This comment was marked as off-topic.

@TheLazron

This comment was marked as off-topic.

@ThomasWaldmann

This comment was marked as off-topic.

TheLazron added a commit to TheLazron/vorta that referenced this issue Mar 6, 2024
@TheLazron

This comment was marked as off-topic.

@ThomasWaldmann

This comment was marked as off-topic.

@borgbase borgbase locked as off-topic and limited conversation to collaborators Mar 7, 2024
@real-yfprojects real-yfprojects changed the title Set supported schemes when opening file dialog. Fix freeze when opening repo on unsupported URL scheme. Apr 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Simple change to start learning code base help wanted This issue is available, comment if you want to fix it type:bug Something doesn't work as intended
Projects
None yet
Development

No branches or pull requests

5 participants