enable the 'sftp' gnomevfs module. blizzard mentioned that this module is included in fedora core 2 as a way to easily browse ones home directory if accessible via ssh.

I was thinking that it would be nice if we could save to this type of url (and smb:// urls as well.) You need an upload channel for that? We talked about this briefly.

right, i'd need to implement nsIUploadChannel. that would be sufficient to make it work with nsWebBrowserPersist. however, our file selection dialog uses nsIFile (i think), so we'd need to rev that code to work with URLs instead, and that is probably the bigger challenge.

Saving to smb and sftp urls won't be an issue until after we hook up the native file picker that's in the queue right now. Once that happens we can take a crack at the apis to make saving work.

i tried out the sftp module from garnome-0.30.1, and i have to say that it is very nice. i think i'd like to morph this bug into adding a whitelist of supported protocols (which by default would include smb: and sftp:).

one interesting (sucky) thing about the sftp build that i used: if a site requires you to login, then you get prompted in the terminal from which you launched moz.

Strange. At least in really recent versions it should pop up a dialog and ask you for the password (there's an external key manager that I'm pretty sure handles this kind of thing.)

since i'm just running garnome-0.30.1 on top of redhat 9, chances are i've not got something configured right.

this patch enables support for sftp by default. i've also added support for reading a whitelist of enabled gnomevfs protocols from preferences. that way, a user could configure moz to work with other gnomevfs protocols.

Comment on attachment 143742 [details] [diff] [review] v1 patch nevermind. this patch has a bug.

revised protocol matching algorithm.

Comment on attachment 143753 [details] [diff] [review] v1.1 patch hmm, this means that I can't change the whitelist at runtime... (you init it in ::Init and don't add an observer...) r-, per irc discussion

ok, now with a pref observer + string iterators.

Comment on attachment 143756 [details] [diff] [review] v1.2 patch Would it be better to parse the string once, in InitSupportedProtocolsPref, and put the protocols into a list, rather than reparse the string on each URI load?

> Would it be better to parse the string once, in InitSupportedProtocolsPref, and > put the protocols into a list, rather than reparse the string on each URI load? I thought about it, but the list is small (always will be) and the parsing is cheap. I'm not sure it's worth the overhead of a nsCStringArray (or whatever data structure). That said, I don't have that strong of an opinion on this. If you'd rather I generate a list, no problem.

Comment on attachment 143756 [details] [diff] [review] v1.2 patch True, the list will be small. >+NS_IMETHODIMP >+nsGnomeVFSProtocolHandler::Observe(nsISupports *aSubject, >+ const char *aTopic, >+ const PRUnichar *aData) >+{ >+ if (strcmp(aTopic, NS_PREFBRANCH_PREFCHANGE_TOPIC_ID) == 0) { This isn't consistent with brace style elsewhere in this file. sr=bryner with that fixed.

This thread makes me worried: http://mail.gnome.org/archives/gnome-vfs-list/2004-March/msg00056.html Apparently, the sftp gnome-vfs module automatically replys with "yes" to any SSH prompt. That has grave security implications even though most users probably always answer yes to those SSH questions. There is no prompting API that we can implement to show these questions to our users, so I think it might be unwise to enable sftp by default in Mozilla at this time. I would, however, like to get the patch in that allows users to enable sftp (and other gnome-vfs protocols) using the prefs. Anyone have thoughts on this?

I agree with darin. I might have opposed this going in at all if it was implemented as a bool pref (it would be easy, and quite conceivable for a "power" mozilla user to find a bool pref in about:config pertaining to gnome and ftp (if they quickly glance and overlook the 's' in front of 'ftp') and switch it on without realizing the implications of doing so). Since this is a comma separated string pref, I don't mind it as much, as it is quite unlikely that they will add sftp to the supported list without a knowledge of what sftp is, and hopefully in their quest to see what gnomevfs protocols we support they will learn of the potential pitfalls of this one.

Maybe you should look at the code instead of reading some mailing list thread. It doesn't automatically reply yes.

> Maybe you should look at the code instead of reading some mailing list thread. > It doesn't automatically reply yes. You are correct, but the mailing list thread made it sound like they were going to change it to automatically reply yes for Gnome 2.6. Hopefully, that won't happen. So, do we enable sftp and simply trust that the sftp maintainers won't do something silly?

The version of 'sftp' that I'm testing (gnome-vfs-sftp-0.1.2.tar.gz) seems to allow ssh to block on stdin waiting for input from the user. this results in blocking one of necko's i/o threads indefinitely :-( sftp needs to either answer 'no' to these prompts or issue some kind of dialog. otherwise, it just seems like trouble to try to enable it in mozilla.

gnome-vfs-sftp-0.1.2 is much **** and buggy compared to the one on gnome-vfs 2.5.x.

ah, ok. i was just testing whatever came with garnome-0.30.1. i'm most interested in testing whatever will ship with gnome 2.6, which is why i was testing with garnome. it seems i should upgrade to garnome-2.5.92.

ok, great. the newer version of sftp returns an "Access Denied" error when confronted with a yes/no prompt. a standard callback that we could implement would be better, but that works for me!

Comment on attachment 143756 [details] [diff] [review] v1.2 patch requesting approval for mozilla 1.7. when Gnome 2.6 ships, this patch will enable us to take advantage of the sftp protocol that makes it easy for Linux users to browse any filesystem to which they have ssh access. this patch also allows user to configure a preference to enable other gnome-vfs protocols.

Comment on attachment 143756 [details] [diff] [review] v1.2 patch a=asa (on behalf of drivers) for checkin to 1.7

fixed-on-trunk for 1.7 final.