Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[url_launcher] Error handling when URL cannot be parsed with Uri.parse #4365

Merged
merged 8 commits into from
Sep 23, 2021
Merged

[url_launcher] Error handling when URL cannot be parsed with Uri.parse #4365

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8f8ed7d
[url_launcher] Error handling when URL cannot be parsed with Uri.parse
Baw-Appie Sep 21, 2021
7f89725
[url_launcher] Add tests for Microsoft Remote Desktop URL
Baw-Appie Sep 22, 2021
6a7b272
[url_launcher] Fix try block to catch only FormatException
Baw-Appie Sep 22, 2021
2f7d786
[url_launcher] Formatting test codes
Baw-Appie Sep 22, 2021
4da3eb7
[url_launcher] Update version to 6.0.12
Baw-Appie Sep 22, 2021
de9b86b
[url_launcher] Fix empty_catches warning
Baw-Appie Sep 22, 2021
0bde82d
[url_launcher] Use Uri.tryParse instead of try-catch
Baw-Appie Sep 22, 2021
3751fe9
[url_launcher] Update tests name
Baw-Appie Sep 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/url_launcher/url_launcher/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 6.0.12

* Fixed an error where 'launch' method of url_launcher would cause an error if the provided URL was not valid by RFC 3986.

## 6.0.11

* Update minimum Flutter SDK to 2.5 and iOS deployment target to 9.0.
Expand Down
8 changes: 6 additions & 2 deletions packages/url_launcher/url_launcher/lib/url_launcher.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,12 @@ Future<bool> launch(
Brightness? statusBarBrightness,
String? webOnlyWindowName,
}) async {
final Uri url = Uri.parse(urlString.trimLeft());
final bool isWebURL = url.scheme == 'http' || url.scheme == 'https';
bool isWebURL = false;
try {
final Uri url = Uri.parse(urlString.trimLeft());
isWebURL = url.scheme == 'http' || url.scheme == 'https';
} on FormatException catch (_) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, you can just use Uri.tryParse instead of this, and simplify the code. I forgot to see if there was a try version of this API before my last review comments.

  final Uri? url = Uri.tryParse(urlString.trimLeft());
  final bool isWebURL = url != null && (url.scheme == 'http' || url.scheme == 'https');


if ((forceSafariVC == true || forceWebView == true) && !isWebURL) {
throw PlatformException(
code: 'NOT_A_WEB_SCHEME',
Expand Down
2 changes: 1 addition & 1 deletion packages/url_launcher/url_launcher/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ description: Flutter plugin for launching a URL. Supports
web, phone, SMS, and email schemes.
repository: https://github.com/flutter/plugins/tree/master/packages/url_launcher/url_launcher
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+url_launcher%22
version: 6.0.11
version: 6.0.12

environment:
sdk: ">=2.14.0 <3.0.0"
Expand Down
36 changes: 36 additions & 0 deletions packages/url_launcher/url_launcher/test/url_launcher_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,42 @@ void main() {
await launchResult;
expect(binding.renderView.automaticSystemUiAdjustment, true);
});

test('open remote desktop url', () async {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's call this 'open non-parseable URL' so it's clear what the real purpose of this test is.

mock
..setLaunchExpectations(
url:
'rdp://full%20address=s:mypc:3389&audiomode=i:2&disable%20themes=i:1',
useSafariVC: false,
useWebView: false,
enableJavaScript: false,
enableDomStorage: false,
universalLinksOnly: false,
headers: <String, String>{},
webOnlyWindowName: null,
)
..setResponse(true);
expect(
await launch(
'rdp://full%20address=s:mypc:3389&audiomode=i:2&disable%20themes=i:1'),
isTrue);
});

test('cannot open remote desktop url with forceSafariVC: true', () async {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here and below; s/remove desktop/non-parseable/

expect(
() async => await launch(
'rdp://full%20address=s:mypc:3389&audiomode=i:2&disable%20themes=i:1',
forceSafariVC: true),
throwsA(isA<PlatformException>()));
});

test('cannot open remote desktop url with forceWebView: true', () async {
expect(
() async => await launch(
'rdp://full%20address=s:mypc:3389&audiomode=i:2&disable%20themes=i:1',
forceWebView: true),
throwsA(isA<PlatformException>()));
});
});
}

Expand Down