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

[hansmbakker]

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 /:

refit/Refit/RestMethodInfo.cs

Lines 258 to 261 in 47ce83e

	if (!relativePath.StartsWith("/"))
	throw new ArgumentException(
	$"URL path {relativePath} must start with '/' and be of the form '/foo/bar/baz'"
	);

and the samples show things like c.BaseAddress = new Uri("https://api.github.com") (missing /) and Get("/users/{user}") (leading /):

refit/README.md

Lines 15 to 35 in f77f0f3

	```csharp
	public interface IGitHubApi
	{
	[Get("/users/{user}")]
	Task<User> GetUser(string user);
	}
	```
	The `RestService` class generates an implementation of `IGitHubApi` that uses
	`HttpClient` to make its calls:
	```csharp
	var gitHubApi = RestService.For<IGitHubApi>("https://api.github.com");
	var octocat = await gitHubApi.GetUser("octocat");
	```
	.NET Core supports registering via HttpClientFactory
	```csharp
	services
	.AddRefitClient<IGitHubApi>()
	.ConfigureHttpClient(c => c.BaseAddress = new Uri("https://api.github.com"));
	```

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

[Dreamescaper]

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.)

[hansmbakker]

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.

[Dreamescaper]

@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=

[hansmbakker]

I just found some more issues related to this.

Looking at #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:

• Refit requires base address of supplied HttpClient to not end with a slash #1358
• Added refit setting to bypass url verification #1113
• [BUG] URL path must start with '/' and be of the form '/foo/bar/baz' #1105

[aj-scram]

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