Skip to content

Conversation

@praveenkumarkarunanithi
Copy link
Contributor

@praveenkumarkarunanithi praveenkumarkarunanithi commented Jan 8, 2025

Root Cause

The root cause of the issue is the use of Uri.IsWellFormedUriString for validating URLs in the WebView control. This method applies overly strict checks, including the IsWellFormedOriginalString test, which can incorrectly reject valid URLs, especially those containing encoded characters or international characters. This leads to improper handling of certain valid URLs, causing them to be treated as relative paths instead of absolute URLs.

Description of Change

In the WebView control, the URL validation logic in MauiWebView.LoadUrl has been updated to replace Uri.IsWellFormedUriString with Uri.TryCreate. This change ensures that a wider range of valid URLs, including those with encoded and international characters, are correctly handled as absolute URLs.

Issues Fixed

Fixes #26843
Fixes #23767
Fixes #23690
Closes #24002

Tested the behaviour in the following platforms

  • Android
  • Windows
  • iOS
  • Mac

Test case failing on windows. for more information: #27425

Screenshots

Before Issue Fix After Issue Fix
withoutfix withfix

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Jan 8, 2025
@praveenkumarkarunanithi
Copy link
Contributor Author

@dotnet-policy-service agree company="Syncfusion, Inc."

@vishnumenon2684 vishnumenon2684 added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Jan 8, 2025
@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines

This comment was marked as outdated.

@jsuarezruiz
Copy link
Contributor

jsuarezruiz commented Jan 17, 2025

/azp run

@azure-pipelines

This comment was marked as outdated.

@jfversluis jfversluis self-assigned this Jan 20, 2025
@praveenkumarkarunanithi praveenkumarkarunanithi marked this pull request as ready for review January 22, 2025 08:23
@praveenkumarkarunanithi praveenkumarkarunanithi requested a review from a team as a code owner January 22, 2025 08:23
@azure-pipelines

This comment was marked as off-topic.

Copy link
Member

@jfversluis jfversluis left a comment

Choose a reason for hiding this comment

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

Build error

/Users/builder/azdo/_work/3/s/src/Core/src/Platform/WebViewHelper.cs(9,32): error CS1503: Argument 1: cannot convert from 'char' to 'string' [/Users/builder/azdo/_work/3/s/src/Core/src/Core.csproj::TargetFramework=netstandard2.0]

@MartyIX
Copy link
Contributor

MartyIX commented Jan 25, 2025

Build error

/Users/builder/azdo/_work/3/s/src/Core/src/Platform/WebViewHelper.cs(9,32): error CS1503: Argument 1: cannot convert from 'char' to 'string' [/Users/builder/azdo/_work/3/s/src/Core/src/Core.csproj::TargetFramework=netstandard2.0]

IMHO we hit #27003 (comment)

@MartyIX
Copy link
Contributor

MartyIX commented Jan 28, 2025

@praveenkumarkarunanithi IMHO it was OK as it was, the only thing that was necessary was to do:

internal static class WebViewHelper
{
    internal static bool IsRelativeUrl(string url)
    {
        // return !url.StartsWith('/') && !Uri.TryCreate(url, UriKind.Absolute, out _);
        return !url.StartsWith("/", StringComparison.Ordinal) && !Uri.TryCreate(url, UriKind.Absolute, out _);
    }
}

@praveenkumarkarunanithi
Copy link
Contributor Author

@praveenkumarkarunanithi IMHO it was OK as it was, the only thing that was necessary was to do:

internal static class WebViewHelper
{
    internal static bool IsRelativeUrl(string url)
    {
        // return !url.StartsWith('/') && !Uri.TryCreate(url, UriKind.Absolute, out _);
        return !url.StartsWith("/", StringComparison.Ordinal) && !Uri.TryCreate(url, UriKind.Absolute, out _);
    }
}

@MartyIX, @jfversluis This fix is specific to the Android platform, creating a unit test would require significant changes to access the method within the unit test class for android. Instead, we added a UI test with assert validation to ensure the fix works across all platforms. This approach provides comprehensive coverage and validation without extensive refactoring. Let us know if you have further thoughts!

@azure-pipelines

This comment was marked as outdated.

@jsuarezruiz

This comment was marked as outdated.

@jsuarezruiz
Copy link
Contributor

jsuarezruiz commented Feb 11, 2025

/azp run

@azure-pipelines

This comment was marked as off-topic.

@jfversluis

This comment was marked as outdated.

@PureWeen
Copy link
Member

PureWeen commented Mar 3, 2025

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

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

WebViewShouldLoadEncodedUrl is failing

@github-project-automation github-project-automation bot moved this from Approved to Changes Requested in MAUI SDK Ongoing Mar 4, 2025
Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

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

WebViewShouldLoadEncodedUrl fails on Android.

image

Assert.Equal() Failure: Values differ\nExpected: Success\nActual: Failure

@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

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

Ok, the tests are passing although I would review the use of VerifyInternetConnectivity.

Uri.IsWellFormedUriString in some scenarios, like including escaped characters, might cause to return false even for a valid URI. Uri.TryCreate is a safer and more robust choice because it both validates the URI and creates the object for further use if needed. In this case, we are creating the Uri object and not using it. Although it is not critical in performance and improves robustness.

/// Checks if internet connection is available by making an HTTP request
/// </summary>
/// <returns>True if internet connection is available</returns>
public static async Task<bool> HasInternetConnection()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary? Can't you use the VerifyInternetConnectivity(); existing method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jsuarezruiz , The existing method (VerifyInternetConnectivity) is added in the UITest project, but since this is a different project (Device Test), I have added the method separately here.

@PureWeen PureWeen modified the milestones: .NET 9 SR6, .NET 9 SR7 Mar 24, 2025
@praveenkumarkarunanithi
Copy link
Contributor Author

praveenkumarkarunanithi commented Mar 25, 2025

Ok, the tests are passing although I would review the use of VerifyInternetConnectivity.

Uri.IsWellFormedUriString in some scenarios, like including escaped characters, might cause to return false even for a valid URI. Uri.TryCreate is a safer and more robust choice because it both validates the URI and creates the object for further use if needed. In this case, we are creating the Uri object and not using it. Although it is not critical in performance and improves robustness.

@jsuarezruiz , Thanks for the feedback! You're correct—I'm currently discarding the created Uri object (out _). The main reason for using Uri.TryCreate here is its improved accuracy in validating URLs containing escaped characters, which Uri.IsWellFormedUriString incorrectly rejects.

@MarkusRodler
Copy link

@PureWeen How many months do we have to wait till this essential bugfix will be merged? This thing is basically done and works as expected (Thanks to @praveenkumarkarunanithi).

@PureWeen PureWeen moved this from Changes Requested to Ready To Review in MAUI SDK Ongoing Mar 28, 2025
@PureWeen PureWeen changed the base branch from main to inflight/current April 22, 2025 22:16
@PureWeen PureWeen merged commit dafe489 into dotnet:inflight/current Apr 22, 2025
125 of 130 checks passed
@github-project-automation github-project-automation bot moved this from Ready To Review to Done in MAUI SDK Ongoing Apr 22, 2025
PureWeen pushed a commit that referenced this pull request Apr 23, 2025
…racters (#27003)

* Fix code and test case commit

* test case code updated

* removed ui tests as it used user name included url

* updated unit test case

* updated fix and unit test case

* updated the new class access specifier.

* Removed unit test descriptions

* Fix updated

* updated the unit test case

* Reverted unit test and added UI test case

* updated test case description

* updating ui test case with ignore windows comment

* updated windows issue url

* updating test case with comments.

* updated the test case as device test

* removed ui test

* Fix build error

* updated device test with conditional skip code

* updating test case with internet connectivity checks.

---------

Co-authored-by: Gerald Versluis <gerald@verslu.is>
github-actions bot pushed a commit that referenced this pull request Apr 28, 2025
…racters (#27003)

* Fix code and test case commit

* test case code updated

* removed ui tests as it used user name included url

* updated unit test case

* updated fix and unit test case

* updated the new class access specifier.

* Removed unit test descriptions

* Fix updated

* updated the unit test case

* Reverted unit test and added UI test case

* updated test case description

* updating ui test case with ignore windows comment

* updated windows issue url

* updating test case with comments.

* updated the test case as device test

* removed ui test

* Fix build error

* updated device test with conditional skip code

* updating test case with internet connectivity checks.

---------

Co-authored-by: Gerald Versluis <gerald@verslu.is>
PureWeen pushed a commit that referenced this pull request May 2, 2025
…racters (#27003)

* Fix code and test case commit

* test case code updated

* removed ui tests as it used user name included url

* updated unit test case

* updated fix and unit test case

* updated the new class access specifier.

* Removed unit test descriptions

* Fix updated

* updated the unit test case

* Reverted unit test and added UI test case

* updated test case description

* updating ui test case with ignore windows comment

* updated windows issue url

* updating test case with comments.

* updated the test case as device test

* removed ui test

* Fix build error

* updated device test with conditional skip code

* updating test case with internet connectivity checks.

---------

Co-authored-by: Gerald Versluis <gerald@verslu.is>
SuthiYuvaraj pushed a commit to SuthiYuvaraj/maui that referenced this pull request May 9, 2025
…racters (dotnet#27003)

* Fix code and test case commit

* test case code updated

* removed ui tests as it used user name included url

* updated unit test case

* updated fix and unit test case

* updated the new class access specifier.

* Removed unit test descriptions

* Fix updated

* updated the unit test case

* Reverted unit test and added UI test case

* updated test case description

* updating ui test case with ignore windows comment

* updated windows issue url

* updating test case with comments.

* updated the test case as device test

* removed ui test

* Fix build error

* updated device test with conditional skip code

* updating test case with internet connectivity checks.

---------

Co-authored-by: Gerald Versluis <gerald@verslu.is>
@github-actions github-actions bot locked and limited conversation to collaborators May 23, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-controls-webview WebView community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration

Projects

Status: Done

9 participants