refit icon indicating copy to clipboard operation
refit copied to clipboard

[Bug]: BaseAddress / relative URL formats - HttpClient style vs Refit style

Open hansmbakker opened this issue 1 year ago β€’ 6 comments

Describe the bug 🐞

Looking at the sources linked below, Refit seems to expect a configuration of HttpClient's `BaseAddress and relative URLs in a way that is different than how HttpClient is meant to be used.

If I misunderstood those sources - please close this issue.

Unfortunately HttpClient is not "smart" about combining the BaseAddress with relative urls and e.g. if you configure the base address with a trailing slash while also configuring the relative urls with a leading slash, it will result in a double // which results in a 404 error at runtime.

Actual behavior

Refit expects the paths to start with a /: https://github.com/reactiveui/refit/blob/47ce83ea8cc5e57a00d567158d983e0097e59cd9/Refit/RestMethodInfo.cs#L258-L261

and the samples show things like c.BaseAddress = new Uri("https://api.github.com") (missing /) and Get("/users/{user}") (leading /): https://github.com/reactiveui/refit/blob/f77f0f39a89323a1566182e2135a0d30382a22a0/README.md?plain=1#L15-L35

Expected behavior

However, the expected HttpClient usage seems to be this

  • the BaseAddress is expected to end with a /
  • relative URLs are expected to start without a /

Therefore, the expected situation is either:

  • Refit following how HttpClient is intended to be used, or
  • clear documentation that Refit is doing it different for a reason.

Sources

  • https://developercommunity.visualstudio.com/t/httpclient-not-smart-about-combining-baseaddress-w/1592519
  • https://stackoverflow.com/questions/23438416/why-is-httpclient-baseaddress-not-working

Step to reproduce

  1. Follow the Refit samples, but configure the BaseAddress as https://api.github.com/ and the GET path as users/{user}
  2. See an exception
  Message: 
    System.ArgumentException : URL path users/{user} must start with '/' and be of the form '/foo/bar/baz'

  Stack Trace: 
    RestMethodInfoInternal.VerifyUrlPathIsSane(String relativePath) line 259

Reproduction repository

https://github.com/reactiveui/refit

hansmbakker avatar Sep 03 '24 12:09 hansmbakker

if you configure the base address with a trailing slash while also configuring the relative urls with a leading slash, it will result in a double // which results in a 404 error at runtime.

That's actually not true, there would be no double slashes. On the contrary - it will remove part of the base url.

HttpClient actually is smart about combining baseUrl and relativeUrl, it's just not obvious if you don't know about it. But - it is based on RFC, and it matches other implementations, like javascript's URL.

That's said, I agree that it's strange that Refit doesn't allow URLs without leading slashes. (I realise that changing the behavior when trailing slash is present would be a huge breaking change.)

Dreamescaper avatar Sep 04 '24 13:09 Dreamescaper

That's actually not true, there would be no double slashes. On the contrary - it will remove part of the base url.

That's not the behavior I saw - if I configure

  • the BaseAddress as https://somehostname/somepath/rest/v1/
  • Refit as [Get("/inspection_results")]

Then the URL that is called by Refit is https://somehostname/somepath/rest/v1//inspection_results.

I specifically configured the BaseAddress with the trailing slash because of the two pages I linked in the sources section in this issue.

(I realise that changing the behavior when trailing slash is present would be a huge breaking change.)

I agree - if this was documented better then that would already help a lot.

hansmbakker avatar Sep 04 '24 13:09 hansmbakker

@hansmbakker Yeah, I meant HttpClient's behavior, not Refit's. HttpClient would invoke https://somehostname/inspection_results in this case:

https://sharplab.io/#v2:C4LgTgrgdgNAJiA1AHwAICYCMBYAUHgNwEMwACAIyIGcBTAVTABtSBeUqGgd1IYEsAKAEQALYMAAOVEAHppVAPYBbGsPlVgUIsrlKa4osGHSwNddIKZpggJQBuQiVInGB3gXpNW7LjzADKtAyMMKSC0rxQVOI0AMbAvPJQAPomVBCMwFQ29riomACc/M6u7kEAdAAq8gDKwH5QAOb81nZAA=

Dreamescaper avatar Sep 04 '24 13:09 Dreamescaper

I just found some more issues related to this.

Looking at https://github.com/reactiveui/refit/issues/1358 (which I now see my issue is a duplicate of), people mention that Refit is not RFC compliant because of this behavior.

A few years ago, changing this behavior was refused and using a custom delegating HttpHandler was recommended.

If this could be reconsidered, or if this could be documented better, it would be really helpful.

Related issues:

  • https://github.com/reactiveui/refit/issues/1358
  • https://github.com/reactiveui/refit/pull/1113
  • https://github.com/reactiveui/refit/issues/1105

hansmbakker avatar Sep 04 '24 13:09 hansmbakker

lol I just was tearing my hair out yesterday about HttpClient not working correctly at it's cause of it needing the ending '/'

Honestly, I prefer Refits way though as it makes more intuitive sense to me. Additionally, at least Refit immediatly throws an error telling you the problem.

HttpClient will work with GetAsJsonAsync using the wrong way and lead you on a merry chase on why POST is failing. Plus, if you use HttpClientExtendedLogging ? Well, that will also cause POST to work cause of how it manipulated the request uri internally I guess

RFC compliance is not needed; clear errors and docs are, so I would say MSFT is the one that needs to step up here, personally. Though, I do think that the fact that Refit uses the HttpClientBuilder can cause confusion between the two, so it would be nice for consistency if it was supported

Just my two cents as a dev that was spending way too much time on this yesterday :P

aj-scram avatar Sep 26 '24 18:09 aj-scram

I guess it would be nice if Refit trimmed the trailing slash automatically when joining to the routes, but I just do this myself when using typed http client for refit

services.AddRefitClient<IMyApiClient>()
  .ConfigureHttpClient((services, client) =>
  {
    var options = services.GetRequiredService<MyApiOptions>();
    // remove trailing slash as the refit routes are prefixed with a slash
    client.BaseAddress = new Uri(options.BaseUri.TrimEnd('/'));
    client.DefaultRequestHeaders.Add(options.ApiKeyHeader, options.ApiKey);
});
public interface IMyApiClient
  {
    [Get("/something/{id}")]
    Task<Entity> GetSomething(string id);

StevenQuickGS1 avatar Dec 08 '24 21:12 StevenQuickGS1