search

foxyproxy / browser-extension

Version 8 and above. Browser extension source code for Firefox, Chrome, and other Chromium-based browsers
GNU General Public License v2.0
198 stars 29 forks source link

empty proxy username/password becomes "undefined" upon unpgrade 7.5.1 --> 8.7 #103

Closed bugfood closed 6 months ago

bugfood commented 6 months ago

There is an issue with the configuration upon upgrade. If there is a proxy configured without a username/password, then both fields become the string undefined upon upgrade. For some reason, this doesn't fully take effect until the first time the configuration is saved--at which point the use of the proxy stops working.

Steps to reproduce:

  1. Start with a fresh firefox profile.
  2. Install FoxyProxy 7.5.1 from https://addons.mozilla.org/en-US/firefox/addon/foxyproxy-standard/versions/?utm_content=search&utm_medium=referral&utm_source=addons.mozilla.org
  3. Import a proxy: socks5://localhost:8801?title=01&proxyDns=false
  4. Upgrade to FoxyProxy 8.7 (go to about:addons, then click the gear, then click "check for updates" -- firefox will update FoxyProxy automatically by default).
  5. Enable the proxy and test browsing; everything should still work ok.
  6. Within the FoxyProxy settings, configure the proxy. Note that the "username" and "password" fields are set to the string undefined.
  7. Click the "Save" button.
  8. Test browsing; firefox is now "Unable to connect".

Thanks, Corey

erosman commented 6 months ago

https://github.com/foxyproxy/firefox-extension/blob/a3598e1c7f0237ee50e8a13a59dec265462c38a6/src/scripts/import-proxy-list.js#L146

While it is a bug, it is bug for v7.5.1 that sets the username & password to undefined, and there is no point in fixing it.

During the migration to v8, the undefined is converted to "undefined" text string and the username & password would be set to "undefined".

AFA v8, it isn't feasible to change the username/password based on the text since undefined can be a valid username or password.

bugfood commented 6 months ago

Ah, ok.

AFA v8, it isn't feasible to change the username/password based on the text since undefined can be a valid username or password.

That's true, but I suggest it might be worthwhile to detect when undefined is used as both the username and password, and convert that to an empty username/password, especially if this can be a one-time check upon upgrade. I would guess that the set of users who actually use undefined as both a username and a password is small relative to the set of users who use proxies without a username and password.

The tricky part was the sequence of events:

  1. I changed a setting--and proxying broke.
  2. I changed the setting back--and proxying was still broken.

Thanks either way, Corey

erosman commented 6 months ago

It is fixed for v8.9.

bugfood commented 6 months ago

Thank you.