[core-http] URLBuilder struggles with parametrized URL and parametrized path

[joheredi]

We're facing a problem with the Tables SDK that I think I have been able to isolate to core-http's URLBuilder.

It seems like having the base URL with a path cause troubles, for example:

describe("path parameters", () => {
  it.only("Test baseUrl with parameter", () => {
    const urlBuilder = URLBuilder.parse("{url}");
    urlBuilder.appendPath("{param}");
    urlBuilder.replaceAll("{url}", "http://localhost:3000/param1");
    urlBuilder.replaceAll("{param}", "param2");
    const url = urlBuilder.toString();
    assert.strictEqual(url, "http://localhost:3000/param1/param2");
  });
});

In this example, I would expect it to end up being http://localhost:3000/param1/param2 however it turns out to dismiss the {param} and ends up as http://localhost:3000/param1

However, if the base URL doesn't have any path elements it works fine

it.only("Test baseUrl with parameter", () => {
    const urlBuilder = URLBuilder.parse("{url}");
    urlBuilder.appendPath("{param}");
    urlBuilder.replaceAll("{url}", "http://localhost:3000");
    urlBuilder.replaceAll("{param}", "param2");
    const url = urlBuilder.toString();
    assert.strictEqual(url, "http://localhost:3000/param2");
  });

The scenario in the Tables SDK where this happens is when targeting the Azure Storage Emulator, in the regular scenario where we hit the service the base url is in this format https://<ACCOUNT_NAME>.table.core.windows.net so it works fine. However when targeting the Storage Emulator we have an url like this http://localhost:10002/<ACCOUNT_NAME> so adding the Tables path in the first case is no issue, in the second scenario it runs in the issue above.

This issue seems to be the cause of #12753 and blocking support for #13118 and #13100

Unfortunately, other places seem to rely on this behavior so changing it may break other libraries. I'm filing this issue to keep track of this problem and for starting up a discussion on what improvements can be made around this.

/cc: @xirzec

[jeremymeng]

So when we parse "{url}", URLBuilder set its host to {url}, which isn't 100% correct as a url can contain all possible components. later after replacement the new host now contains another path component /param1. Then setting the new host will cause the old path of {param} to be overwritten in the state machine. I need to think more about this.

[xirzec]

So when we parse "{url}", URLBuilder set its host to {url}, which isn't 100% correct as a url can contain all possible components. later after replacement the new host now contains another path component /param1. Then setting the new host will cause the old path of {param} to be overwritten in the state machine. I need to think more about this.

Yeah, basically if you feed URLBuilder a url that already contains a path and then set it again later it replaces it instead of concatenating it. We rely on this behavior in other client libs, so we can't just fix it directly.

[jeremymeng]

Given that there's a workaround I am closing this for now.