With Python 3.5.0rc2 (tested with both Git BASH and Cmder on Windows 8):

$ python3
>>> import webbrowser
>>> webbrowser.open_new('http://example.com/?a=1&b=2')
'b' is not recognized as an internal or external command,
operable program or batch file.
True

The opened page is http://example.com/?a=1 rather than http://example.com/?a=1&b=2.

Trying to open a URL with only one field works as expected.

Thank you for reporting this.

I see that the Windows browser class uses shell=True, and that is wrong from a security standpoint.

This appears to be a regression from 3.4, introduced by issue 8232.  Since this is a security regression there either needs to be a fix or that changeset should be backed out.

It'll have to be backed out. There may be a fix to salvage some of the functionality, but it's certainly too significant at this stage.

Here's an alternative to backing out the change, and it's simpler than I expected when I said it would be too much for 3.5.0.

We add an 'arguments' parameter to os.startfile and use that instead of subprocess.call(shell=True). The underlying ShellExecute call does not do any argument processing, so you can pass through any arguments you like.

In the attached patch, I only added the argument for when Unicode strings are used, since byte strings are deprecated, but it's fairly trivial to add it to both. I'll add a backout patch next so they can be compared.

Patch for backing out #8232's changes.

Patch 1 also requires a minor update to Doc\library\os.rst:

-.. function:: startfile(path[, operation])
+.. function:: startfile(path[, operation[, arguments]])
...
+*arguments* is passed to the underlying :c:func:`ShellExecute`
+call. Its format is determined by the target file and operation.

Marking this as deferred, as I'm not convinced I should ship either of those patches in 3.5.0rc3.

Does that mean not shipping either of them in 3.5.0 at all? Do you need convincing or a simpler patch?

You have to ship one of them by ("one of them" being either the fix or the backout) in 3.5.0, Larry, otherwise you are introducing a security vulnerability into 3.5 that doesn't exist in 3.4.  If you don't ship it in rc3, then there's no chance that 3.5.0 will be unchanged from rc3, which is the ideal we should be aiming for (that's why it's called a "release *candidate*").

I want to ship something, but I don't think it'll be either of those patches in their current form.

Maybe I'm dense, but I don't feel like I understand these patches.  They have very different approaches.

The first one attempts to rehabilitate the patch by running the browser directly instead of using "start"?  Do the features added in issue 8232 still work?

The second one just backs out the feature completely?  The features added in issue 8232 are gone?

And both patches switch from using subprocess.call(shell=True) to os.startfile(), which is the security hole David is talking about, which isn't actually what this bug is about.

Do both patches fix the erroneous behavior observed by the original bug report, with "&" being interpreted as a shell command separator?

subprocess with shell=True turns it into a "cmd.exe /C "start chrome.exe ..."" type command, which means the arguments will use shell parsing (e.g. > for redirection, & for multiple commands, etc.)

"start" in cmd.exe behaves the same as os.startfile, but can also accept arguments. Patch 1 lets startfile accept arguments, so we don't have to go via cmd.exe to launch the browser (we're still resolving it via the shell, so the behaviour is the same, but the arguments are never interpreted using shell rules). All the functionality of #8232 is still there and still works.

Patch 2 is a rollback of #8232, so we ship the same functionality as in Python 3.4. Because this only supports the default browser and does not support selecting between new tab/window, it can go back to using os.startfile without arguments.

A third option would be to find some other way to discover the full path to each browser and launch it without using start/os.startfile. (This is what I was thinking originally when I said it would be too big a change.)

To be more specific, with patch 1 applied:

subprocess.call("start file a&b>x", shell=True)

is equivalent to typing the following at a command prompt:

start file a & b > x

That is, "start file a" and then do "b", redirecting the output from "b" to a file named "x".

With the change to os.startfile, we can write it as this:

os.startfile("file", "open", "a&b>x")

Which matches what we intended above. (startfile implies "start <first argument>", and passes the third argument to the launched program without modification.)

Oh, and the "start" is necessary because, while the Windows kernel can only resolve "chrome.exe" if it appears on PATH, the Windows shell has some other ways to resolve it.

By using ShellExecute (via 'start' or startfile), we can let the OS find it rather than having to find it ourselves.

So, whatever the security hole is that subprocess.call(shell=True) leaves open, os.startfile() doesn't have?  os.startfile() doesn't use a shell?  (How does it find the full path to the executable?)

Correct.

os.startfile uses ShellExecute (https://msdn.microsoft.com/en-us/library/windows/desktop/bb762153.aspx), which is the same API that the shell uses for the 'start' command. So by using os.startfile we get the same behaviour, but we're calling in after the terminal would have done its own parsing (for piping/redirection/etc.).

Well, so, what do you recommend I do here?

Rollback.

I'm not 100% confident in patch 1 (too many things I can't predict) and with only a week it probably won't get enough testing to flush out other surprises.

I guess now I've been that definitive I'll go make you a PR :)

If someone (perhaps Brandon?) is willing to thoroughly validate patch 1 we might be able to consider it for 3.5.1 (the only API change is to startfile() - the webbrowser API is already there, it just doesn't report much). Otherwise, it'll have to be 3.6. (But will it get any more tested there? Not so sure...)

PR is: https://bitbucket.org/larry/cpython350/pull-requests/20/issue-25005-backout-fix-for-8232-because/diff

Backout pull request merged, please forward-merge, thanks!

New changeset aa60b34d5200 by Steve Dower in branch '3.5':
Issue #25005: Backout fix for #8232 because of use of unsafe subprocess.call(shell=True)
https://hg.python.org/cpython/rev/aa60b34d5200

New changeset 7d320c3bf9c6 by Larry Hastings in branch '3.5':
Merged in stevedower/cpython350 (pull request #20)
https://hg.python.org/cpython/rev/7d320c3bf9c6