Flurl: Leading slash broke RequestUri in HttpClient with BaseAddress

Created on 11 Nov 2020  路  8Comments  路  Source: tmenier/Flurl

Hi!

I've tried to bump Flurl and faced with broken RequestUri.

My HttpClient has BaseAddress with some relative path like http://fake.com/api/
I compose Uri and Flurl adds leading / when I use AppendPathSegment for example "path1".AppendPathSegment("path2")
As a result, RequestUri equals http://fake.com/path1/path2 but expected http://fake.com/api/path1/path2

See https://github.com/ch1seL/FlurlTest I reproduced it there

Could I use relative paths in BaseAddress or it is a bad practice?

bug

Most helpful comment

@ch1seL Perfect, that's exactly what I was after. I didn't doubt there might be an issue lurking somewhere, it just got rather obfuscated in those tests. :)

I agree that the Flurl 2 behavior is more correct and that this is a regression bug. I will get it fixed. (As a side note, if you use Flurl.Http to make your calls rather than HttpClient, it shouldn't matter.)

All 8 comments

Could you point me to the specific code and not the whole repo?

Could you point me to the specific code and not the whole repo?

this is a little solution special for testing this bug =)
there are two projects(depend on Flurl2 and FLurl3) with one test it uses GetRequestUrl

Result:
Flurl2Tests passed
Flurl3Tests failed

I understand what it is what it is but if you instead provided a minimal repro I could probably answer right now. What's happening in this test is confusing and needs to be reduced. I have no idea why you're doing a fake HTTP call to get the actualUrl string for example, but that shouldn't be necessary to repro a bug in something that only deals with string manipulation. Get it down to a minimal example comparing a _hard-coded_ expected URL to one that's composed using AppendPathSegment and I'll take a look.

I do fake HTTP call through HttpClient cause it is correlated to HttpClient logic
Ok, this minimal example without Flurl

            var httpClient = new HttpClient()
            {
                BaseAddress = new Uri("http://fake.com/api/")
            };
            httpClient.GetAsync("path"); // RequestUri is http://fake.com/api/path
            httpClient.GetAsync("/path"); // RequestUri is  http://fake.com/path

Flurl 3 returns path starting with / but Flurl 2 don't

I meant a minimal example demonstrating the Flurl bug, but let's forget about that and go back to your original tests. Your GetRequestUrl method takes 2 stings and makes a call using HttpClient. It doesn't use Flurl at all. The only place you're using Flurl is to construct that 2nd string (requestUri), so if you're trying to demonstrate that there's a regression bug with Flurl, all you should need to do is inspect the value of requestUri and determine if it's what you expect.

From your test:

```c#
// what is the value of requestUri here? is it what you expect? is it different than Flurl2?
Url requestUri = path1.AppendPathSegment(path2);

// you're completely done using Flurl here, so this part doesn't even matter
var actualUrl = await GetRequestUrl(baseAddress, requestUri);
```

I can confirm that we have the same issue after upgrading Flurl to version 3.0.0. No code changes, and now LocalPath in BaseAddress is not respected for some reason. It is reasonable to suspect to Flurl since that is the only thing that was changed.

Flurl 2:

"path1".AppendPathSegment("path2") // "path1/path2"

Flurl 3:

"path1".AppendPathSegment("path2") // "/path1/path2"

@ch1seL Perfect, that's exactly what I was after. I didn't doubt there might be an issue lurking somewhere, it just got rather obfuscated in those tests. :)

I agree that the Flurl 2 behavior is more correct and that this is a regression bug. I will get it fixed. (As a side note, if you use Flurl.Http to make your calls rather than HttpClient, it shouldn't matter.)

Was this page helpful?
0 / 5 - 0 ratings