search

OData / odata.net

ODataLib: Open Data Protocol - .NET Libraries and Frameworks
https://docs.microsoft.com/odata
Other
687 stars 349 forks source link

URI validation error (Uri.IsWellFormedUriString()) #2374

Open AntiGuideAkquinet opened 2 years ago

AntiGuideAkquinet commented 2 years ago

Because of an issue in the .NET Framework this line prevents me from creating a DataServiceContext with a valid URI flagging it as invalid incorrectly. https://github.com/OData/odata.net/blob/acc2c70810eb9a12ef53bb640a2718d9d4db154e/src/Microsoft.OData.Client/UriResolver.cs#L180

The URI in question is http://192.168.0.1:1234/Instance/ODataV4/Company('123-Customer Place Süd-Ost')/ It is perfectly valid. Even the percent encoded version doesn't work. The issue seems to occur only when a space and a non ASCII character like ä is present.

As the issue is planned to be fixed in .NET 7 and not in previous versions I hope that you can provide a workaround as this is the default OData URI scheme for Microsofts Business Central for example. Maybe the check could be optional or done through a developer provided custom function.

KenitoInc commented 2 years ago

Kindly provide a stacktrace so that we can investigate this issue.

AntiGuideAkquinet commented 2 years ago 
using Microsoft.OData.Client;

new DataServiceContext(new Uri("http://192.168.0.1:1234/Instance/ODataV4/Company('123-Customer Place Süd-Ost')/"), ODataProtocolVersion.V4);

results in

Unhandled exception. System.ArgumentException: Expected an absolute, well formed http URL without a query or fragment. (Parameter 'serviceRoot')
   at Microsoft.OData.Client.UriResolver.ConvertToAbsoluteAndValidateBaseUri(Uri& baseUri, String parameterName)
   at Microsoft.OData.Client.UriResolver.CreateFromBaseUri(Uri baseUri, String parameterName)
   at Microsoft.OData.Client.DataServiceContext..ctor(Uri serviceRoot, ODataProtocolVersion maxProtocolVersion, ClientEdmModel model)
   at Microsoft.OData.Client.DataServiceContext..ctor(Uri serviceRoot, ODataProtocolVersion maxProtocolVersion)
   at Program.<Main>$(String[] args) in C:\Users\MyUser\RiderProjects\ODataUri\ODataUri\Program.cs:line 3
KenitoInc commented 2 years ago

@AntiGuideAkquinet We initialize the DataServiceContext with the ServiceRoot. You are using a full url with query options. That's what is causing the exception. See docs

AntiGuideAkquinet commented 2 years ago

@KenitoInc I don't know if fully understand. My endpoints + filter etc. would follow after the DataServiceContext's URI. For example http://192.168.0.1:1234/Instance/ODataV4/Company('123-Customer Place Süd-Ost')/UsersTableEndpoint?$filter=Id eq 1

My data access via OData is implemented in a context class derriving from DataServiceContext consisting of a few DataServiceQuerys that are later queried via LINQ.

public sealed class BcOdataContext : DataServiceContext
    {
        public const string USER = "UsersTableEndpoint";

        public BcOdataContext(Uri serviceRoot, ODataProtocolVersion maxProtocolVersion) : base(serviceRoot, maxProtocolVersion)
        {
            //serviceRoot is "http://192.168.0.1:1234/Instance/ODataV4/Company('123-Customer Place Süd-Ost')/"
            Users = CreateQuery<BcOdataTables.UsersTableEndpoint>(USER);
        }

        public DataServiceQuery<BcOdataTables.UsersTableEndpoint> Users { get; }
    }

Are you suggesting I use http://192.168.0.1:1234/Instance/ODataV4/ as the serviceRoot and put Company('123-Customer Place Süd-Ost')/UsersTableEndpoint in the DataServiceQuery? The company part is used as a way to specify a tenant by Business Central.

AntiGuideAkquinet commented 2 years ago

I tried to implement a basic workaround that fixes the issue for me here. There are very likely better and more elegant ways to implement this but I believe it shows what a simple fix that would not break compatibility with current integrations could look like.

KenitoInc commented 2 years ago

This sample code works for me.

Container dsc = new Container(new Uri("http://localhost:5000/odata"));

// GET http://localhost:5000/odata/Customers(1)/MainAddress
DataServiceQuery<Customer> customers = dsc.CreateQuery<Customer>("Customers(1)");

foreach(var customer in customers)
{
    Console.WriteLine(customer.MainAddress);
}
AntiGuideAkquinet commented 2 years ago

As stated in my first comment:

The issue seems to occur only when a space and a non ASCII character like ä is present.

To reproduce the issue you'll have to use my URI or another one triggering the same .NET bug

chrisspre commented 2 years ago

Uri.IsWellFormedUriString("http://192.168.0.1:1234/Instance/ODataV4/Company('123-Customer ')", UriKind.Absolute); returns false, But Uri.IsWellFormedUriString("http://192.168.0.1:1234/Instance/ODataV4/Company('123-Customer%20')", UriKind.Absolute); returns true.

AntiGuideAkquinet commented 2 years ago

@chrisspre That ist true but misses the point that

a space and a non ASCII character

must be present.

The problem is that Uri.IsWellFormedUriString("http://192.168.0.1:1234/Instance/ODataV4/Company('123-Cust%C3%B6mer%20')", UriKind.Absolute); and Uri.IsWellFormedUriString("http://192.168.0.1:1234/Instance/ODataV4/Company('123-Custömer ')", UriKind.Absolute); both return false while they are valid.

@KenitoInc Can a hotfix like this be implemented soon? This issue forces all people interfacing with Business Central/Navision that have spaces and non ASCII characters in their tenant names to not use OData or fork the repo.

chrisspre commented 2 years ago

Apologies for not giving more context for my short note. What I intended to say was, that this is a base class library and we would need to file a bug with them. Having the two additional examples is useful.

AntiGuideAkquinet commented 2 years ago

It is already filed here and scheduled for .NET 7. My request is for a fix considering .NET versions <7.0 that can be used in the meantime and on older versions. The possible solutions coming to my mind are:

chrisspre commented 2 years ago

Thanks for the .net 7 bug. It is not sufficient to fix the IsWellFormed check (or provide a clay to inject code to do the check). The URL parser also needs to be able to parse and identify the various elements of the URL.

From: AntiGuideAkquinet @.> Sent: Tuesday, May 3, 2022 9:56:04 PM To: OData/odata.net @.> Cc: Christof Sprenger @.>; Mention @.> Subject: Re: [OData/odata.net] URI validation error (Uri.IsWellFormedUriString()) (Issue #2374)

It is already filed herehttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fruntime%2Fissues%2F21626&data=05%7C01%7Cchristof.sprenger%40microsoft.com%7C1c64d47d213042d5273608da2d8a6520%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637872369681056835%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=MJe%2Bza0PVknDBMUcugC0te3rZBB5Hvxntj3FTya5Ncc%3D&reserved=0 and scheduled for .NET 7. My request is for a fix considering .NET versions <7.0 that can be used in the meantime and on older versions. The possible solutions coming to my mind are:

— Reply to this email directly, view it on GitHubhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FOData%2Fodata.net%2Fissues%2F2374%23issuecomment-1116928505&data=05%7C01%7Cchristof.sprenger%40microsoft.com%7C1c64d47d213042d5273608da2d8a6520%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637872369681056835%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=h30sPCwzmjVihp3T6wx%2F2Y8rVYFh%2FQORfaxki4UM8lM%3D&reserved=0, or unsubscribehttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAMVCUHHGLZWNJPQZU3HLAQ3VIH7OJANCNFSM5THVXTJA&data=05%7C01%7Cchristof.sprenger%40microsoft.com%7C1c64d47d213042d5273608da2d8a6520%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637872369681056835%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2BG23L7fBShiTANkaJzUlTn4bePQOunsoOBI%2BY40D0Oo%3D&reserved=0. You are receiving this because you were mentioned.Message ID: @.***>

AntiGuideAkquinet commented 2 years ago

It seem to work perfectly for now. My quick fix is in active use. If there are issues with it I have not discovered them yet.

AntiGuideAkquinet commented 2 years ago

Is there anything I can do to help you to resolve the issue?

AntiGuideAkquinet commented 2 years ago

A contributor of the dotnet runtime suggested using Uri.TryCreate to work around the issue. How would you feel about that as a fix while waiting for it to be implemented in .NET 8?

MihaZupan commented 2 years ago

See https://github.com/dotnet/runtime/issues/72632#issuecomment-1282347554

The OData library should consider dropping this check entirely. As-is, it is checking Uri.IsWellFormedUriString(baseUri.AbsoluteUri, UriKind.Absolute) which should just always be true, as Uri.AbsoluteUri is always generating a well-formed string. That is, this check is doing nothing in this case, except for potentially hitting this false-negative bug.

From what I can tell, OData can safely remove the IsWellFormedUriString check here entirely.

AntiGuideAkquinet commented 1 year ago

When creating this issue, the implementation target for a .NET wide fix was .NET 7. That target has since been moved to .NET 8 and is now pushed out indefinetly:

Sadly, we won't make it in 8.0 due to circumstances. Moving to Future for now.

I want to bring this issue to your attention again to have a chance to remove the necessity for a personal fork for everyone using Microsoft's own Business Central with the go-to webservice, for example. For us, this issue introduces the need to maintain a fork of this library or go back to SOAP as the other web service choice.

The suggested fix, as stated above by @MihaZupan is to remove the check.

tfaller commented 3 weeks ago

We can work around this by using Reflection. Of course this could break with any updates, but it is good enough for now. Tested with 8.0.2

using Microsoft.OData.Client;

var baseUri = new Uri("http://192.168.0.1:1234/Instance/ODataV4/Company('123-Customer Place Süd-Ost')/");

// context with a placeholder URI
var context = new DataServiceContext(new Uri("https://127.0.0.1"), ODataProtocolVersion.V4);

// create UriResolver with the correct URI
var urlResolver = Activator.CreateInstance(
   "Microsoft.OData.Client", "Microsoft.OData.Client.UriResolver", false,
   BindingFlags.NonPublic | BindingFlags.Instance, null,
   [baseUri, null], // UriResolver(...)
   null, null).Unwrap();

// inject the correct UriResolver into the context
typeof(DataServiceContext)
   .GetField("baseUriResolver", BindingFlags.Instance | BindingFlags.NonPublic)
   .SetValue(context, urlResolver);