ChilliCream / graphql-platform

Welcome to the home of the Hot Chocolate GraphQL server for .NET, the Strawberry Shake GraphQL client for .NET and Banana Cake Pop the awesome Monaco based GraphQL IDE.
https://chillicream.com
MIT License
5.07k stars 723 forks source link

Uri deserialization should support relative URIs #7207

Open gregsdennis opened 1 week ago

gregsdennis commented 1 week ago

Product

Strawberry Shake

Version

13.9.4

Link to minimal reproduction

https://github.com/json-everything/json-everything/blob/637895947c44a376b13b4019256b6da1316773e2/tools/GenerateSponsorList/Program.cs#L27

Steps to reproduce

Query against GitHub API:

query {
  user(login:"<username>") {
    ... on Sponsorable {
      sponsors(first: 10) {
        nodes {
          ... on User {
            websiteUrl
          }
          ... on Organization {
            websiteUrl
          }
        }
      }
    }
  }
}

You need a sponsor who's URL is relative, like mysite.io. Apparently GH accepts that.

I can't create a reproduction because it requires authenticated data. The reproduction link I've supplied is the point at which my app fails.

What is expected?

I expect the response to deserialize.

Uri.TryCreate() successfully makes a relative URI.

What is actually happening?

Apparently one of my sponsors has entered a URL like mysite.io as their website, and the ILeafValueParser<Uri> that's generated seems to have a hard time parsing it. Seems it interprets this as a relative URI and expects it to start with a slash.

Exception is GraphQLClientException with this as the message:

The relative uri mysite.io does not start with '/'

Relevant log output

No log output.

Additional context

Per @michaelstaib (in Slack) the serializer is https://github.com/ChilliCream/graphql-platform/blob/main/src/StrawberryShake/Client/src/Core/Serialization/UrlSerializer.cs

gregsdennis commented 3 days ago

I'd like to propose two approaches:

Check for hostname

Use the Uri.CheckHostName() method to check if it's a valid host name. If so, maybe prepend https:// on it and try to parse again.

This approach suits this particular case where some user has entered mysite.io as their website, but it does make the assumption that this is the case.

Support custom parsers

(Maybe this is already done; I can't find it.)

It'd be nice to be able to register a custom converter for a particular type, like you can with System.Text.Json.

michaelstaib commented 3 days ago

Lets remove the check and allow relative URIs.