-
-
Notifications
You must be signed in to change notification settings - Fork 34.5k
gh-137586: Replace 'osascript' with 'open' on macOS in webbrowser #146439
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
base: main
Are you sure you want to change the base?
Changes from 8 commits
00fa6c7
f697cd5
7781033
6066221
d54293f
080197e
fdd6649
8e1eef4
e193626
98dd1d8
bdfc2e6
614078f
c77f4b8
c725927
1316dbf
612d276
386af23
e4e89f1
b513903
aacbceb
a6d76d6
5a4fdad
de9ec77
6ddf67f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -491,10 +491,10 @@ def register_standard_browsers(): | |
| _tryorder = [] | ||
|
|
||
| if sys.platform == 'darwin': | ||
| register("MacOSX", None, MacOSXOSAScript('default')) | ||
|
gpshead marked this conversation as resolved.
|
||
| register("chrome", None, MacOSXOSAScript('google chrome')) | ||
| register("firefox", None, MacOSXOSAScript('firefox')) | ||
| register("safari", None, MacOSXOSAScript('safari')) | ||
| register("MacOSX", None, MacOSX('default')) | ||
| register("chrome", None, MacOSX('google chrome')) | ||
| register("firefox", None, MacOSX('firefox')) | ||
| register("safari", None, MacOSX('safari')) | ||
| # macOS can use below Unix support (but we prefer using the macOS | ||
| # specific stuff) | ||
|
|
||
|
|
@@ -613,8 +613,131 @@ def open(self, url, new=0, autoraise=True): | |
| # | ||
|
|
||
| if sys.platform == 'darwin': | ||
| def _macos_default_browser_bundle_id(): | ||
|
gpshead marked this conversation as resolved.
|
||
| """Return the bundle ID of the default web browser via NSWorkspace. | ||
|
|
||
| Uses the Objective-C runtime directly to call | ||
| NSWorkspace.sharedWorkspace().URLForApplicationToOpenURL() with a | ||
| probe https:// URL, then reads the bundle identifier from the | ||
| resulting NSBundle. Returns None if ctypes is unavailable or the | ||
| lookup fails for any reason. | ||
| """ | ||
| try: | ||
| from ctypes import cdll, c_void_p, c_char_p | ||
| from ctypes.util import find_library | ||
|
|
||
| # NSWorkspace is an AppKit class; load AppKit to register it. | ||
| cdll.LoadLibrary( | ||
| '/System/Library/Frameworks/AppKit.framework/AppKit' | ||
| ) | ||
| objc = cdll.LoadLibrary(find_library('objc')) | ||
| objc.objc_getClass.restype = c_void_p | ||
| objc.sel_registerName.restype = c_void_p | ||
| objc.objc_msgSend.restype = c_void_p | ||
|
|
||
| def cls(name): | ||
| return objc.objc_getClass(name) | ||
|
|
||
| def sel(name): | ||
| return objc.sel_registerName(name) | ||
|
|
||
| # Build probe NSURL for "https://python.org" | ||
| NSString = cls(b'NSString') | ||
| objc.objc_msgSend.argtypes = [c_void_p, c_void_p, c_char_p] | ||
| ns_str = objc.objc_msgSend( | ||
| NSString, sel(b'stringWithUTF8String:'), b'https://python.org' | ||
| ) | ||
|
|
||
| NSURL = cls(b'NSURL') | ||
| objc.objc_msgSend.argtypes = [c_void_p, c_void_p, c_void_p] | ||
| probe_url = objc.objc_msgSend(NSURL, sel(b'URLWithString:'), ns_str) | ||
|
|
||
| # Ask NSWorkspace which app handles https:// | ||
| NSWorkspace = cls(b'NSWorkspace') | ||
| objc.objc_msgSend.argtypes = [c_void_p, c_void_p] | ||
| workspace = objc.objc_msgSend(NSWorkspace, sel(b'sharedWorkspace')) | ||
| if not workspace: | ||
| return None | ||
|
|
||
| objc.objc_msgSend.argtypes = [c_void_p, c_void_p, c_void_p] | ||
| app_url = objc.objc_msgSend( | ||
| workspace, sel(b'URLForApplicationToOpenURL:'), probe_url | ||
| ) | ||
| if not app_url: | ||
| return None | ||
|
|
||
| # Get bundle identifier from that app's NSBundle | ||
| NSBundle = cls(b'NSBundle') | ||
| bundle = objc.objc_msgSend(NSBundle, sel(b'bundleWithURL:'), app_url) | ||
| if not bundle: | ||
| return None | ||
|
|
||
| objc.objc_msgSend.argtypes = [c_void_p, c_void_p] | ||
| bundle_id_ns = objc.objc_msgSend(bundle, sel(b'bundleIdentifier')) | ||
| if not bundle_id_ns: | ||
| return None | ||
|
|
||
| objc.objc_msgSend.restype = c_char_p | ||
| bundle_id_bytes = objc.objc_msgSend(bundle_id_ns, sel(b'UTF8String')) | ||
| return bundle_id_bytes.decode() if bundle_id_bytes else None | ||
| except Exception: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this be more specific? |
||
| return None | ||
|
|
||
| class MacOSX(BaseBrowser): | ||
|
hugovk marked this conversation as resolved.
Outdated
|
||
| """Launcher class for macOS browsers, using /usr/bin/open. | ||
|
|
||
| For http/https URLs with the default browser, /usr/bin/open is called | ||
| directly; macOS routes these to the registered browser. | ||
|
|
||
| For all other URL schemes (e.g. file://) and for named browsers, | ||
| /usr/bin/open -b <bundle-id> is used so that the URL is always passed | ||
| to a browser application rather than dispatched by the OS file handler. | ||
| This prevents file injection attacks where a file:// URL pointing to an | ||
| executable bundle could otherwise be launched by the OS. | ||
|
|
||
| Named browsers with known bundle IDs use -b; unknown names fall back | ||
| to -a. | ||
| """ | ||
|
|
||
| _BUNDLE_IDS = { | ||
|
hugovk marked this conversation as resolved.
|
||
| 'google chrome': 'com.google.Chrome', | ||
| 'firefox': 'org.mozilla.firefox', | ||
| 'safari': 'com.apple.Safari', | ||
| 'chromium': 'org.chromium.Chromium', | ||
| 'opera': 'com.operasoftware.Opera', | ||
|
hugovk marked this conversation as resolved.
|
||
| 'microsoft edge': 'com.microsoft.Edge', | ||
| } | ||
|
|
||
| def open(self, url, new=0, autoraise=True): | ||
| sys.audit("webbrowser.open", url) | ||
| self._check_url(url) | ||
| if self.name == 'default': | ||
| proto, sep, _ = url.partition(':') | ||
| if sep and proto.lower() in {'http', 'https'}: | ||
| cmd = ['/usr/bin/open', url] | ||
| else: | ||
| bundle_id = _macos_default_browser_bundle_id() | ||
| if bundle_id: | ||
| cmd = ['/usr/bin/open', '-b', bundle_id, url] | ||
| else: | ||
| cmd = ['/usr/bin/open', url] | ||
|
gpshead marked this conversation as resolved.
Outdated
|
||
| else: | ||
| bundle_id = self._BUNDLE_IDS.get(self.name.lower()) | ||
| if bundle_id: | ||
| cmd = ['/usr/bin/open', '-b', bundle_id, url] | ||
| else: | ||
| cmd = ['/usr/bin/open', '-a', self.name, url] | ||
| proc = subprocess.run(cmd, stderr=subprocess.DEVNULL) | ||
| return proc.returncode == 0 | ||
|
|
||
| class MacOSXOSAScript(BaseBrowser): | ||
| def __init__(self, name='default'): | ||
| import warnings | ||
| warnings.warn( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we pick a removal version, we can use https://discuss.python.org/t/introducing-warnings-deprecated/14856
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switched to |
||
| "MacOSXOSAScript is deprecated, use MacOSX instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2, | ||
| ) | ||
| super().__init__(name) | ||
|
|
||
| def open(self, url, new=0, autoraise=True): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| Add :class:`!MacOSX` to :mod:`webbrowser` for macOS, which opens URLs via | ||
| ``/usr/bin/open`` instead of piping AppleScript to ``osascript``. | ||
| Deprecate :class:`!MacOSXOSAScript` in favour of :class:`!MacOSX`. | ||
|
secengjeff marked this conversation as resolved.
Outdated
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Fix a PATH-injection vulnerability in :mod:`webbrowser` on macOS where | ||
| ``osascript`` was invoked without an absolute path. The new :class:`!MacOSX` | ||
|
secengjeff marked this conversation as resolved.
Outdated
|
||
| class uses ``/usr/bin/open`` directly, eliminating the dependency on | ||
| ``osascript`` entirely. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3.15
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And precede this
deprecatedblock with aversionaddedfor the new class, and move both afterversionchanged:: 3.13below.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also the
MacOSXOSAScripts in the table above need updating, and should we also addchromeandfirefox(with note (3)) as also returning the new class?