-
Notifications
You must be signed in to change notification settings - Fork 974
[hackerone] consider disabling FTP #14712
Comments
FWIW, I think that Chrome has gone back and forth on this one a bunch of times. FTP within a regular browsing context is a bit of a bugbear. On the other hand, lots of documents are only available via FTP — especially legacy government documents. Leaving FTP support in the browser really lowers the barrier to accessing those materials. Of course, we don't need to be the only way for journalists and so on to access government documents. I'd be pretty comfortable with ripping out FTP for the medium term and building it back as an independent feature with a modern security & privacy mindset later (perhaps much later). |
i think all of these issues are specific to muon + FTP, so when FTP returns in brave-core, it will probably not cause problems like these |
Pushing back to release 5; we'll want to wait until after C68 is merged before merging the Muon changes required for this (see above links to PRs) |
Fixed with brave/muon@25cf424 |
@bsclifton to confirm, when I do those steps on macOS I get this notification banner: And if I click Allow, I am prompted to enter credentials. I entered the creds from https://dlptest.com/ftp-test/ but couldn't login. |
i think the fact that it's prompting to open an external application is correct, since we will no longer handle FTP ourselves. not sure why the credentials don't work but that doesn't seem like a Brave bug. |
@diracdeltas so since I'm being prompted via the notification banner then this one is ok as is (at least on macOS)? cc @kjozwiak so he can check on his machine as well. |
@LaurenWags yes this seems fine |
@diracdeltas this is what happens on Windows with FTP linked to open in Brave. It goes on an infinite loop opening blank pages but never prompting to enter credentials. |
Test plan
Original issue description
https://hackerone.com/reports/378805
related:
https://hackerone.com/bugs?subject=brave&report_id=378809
https://hackerone.com/reports/378864
The text was updated successfully, but these errors were encountered: