Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RequestTransformContext.DestinationPrefix should be read-write #2150

Closed
TrevorDArcyEvans opened this issue May 31, 2023 · 2 comments · Fixed by #2311
Closed

RequestTransformContext.DestinationPrefix should be read-write #2150

TrevorDArcyEvans opened this issue May 31, 2023 · 2 comments · Fixed by #2311
Assignees
Labels
help wanted We will welcome a contribution Type: Bug Something isn't working
Milestone

Comments

@TrevorDArcyEvans
Copy link

Describe the bug

commit: 527fafe

Comments in RequestTransformContext suggest that DestinationPrefix is able to be updated:

     /// <summary>
    /// The outgoing proxy request. All field are initialized except for the 'RequestUri' and optionally headers.
    /// If no value is provided then the 'RequestUri' will be initialized using the updated 'DestinationPrefix',
    /// 'Path', and 'Query' properties after the transforms have run. The headers will be copied later when
    /// applying header transforms.
    /// </summary>
    public HttpRequestMessage ProxyRequest { get; init; } = default!;

but DestinationPrefix cannot be changed:

    /// <summary>
    /// The URI prefix for the proxy request. This includes the scheme and host and can optionally include a
    /// port and path base. The 'Path' and 'Query' properties will be appended to this after the transforms have run.
    /// </summary>
    public string DestinationPrefix { get; init; } = default!;

Question
Is this a bug in the code or the comment?

@TrevorDArcyEvans TrevorDArcyEvans added the Type: Bug Something isn't working label May 31, 2023
@Tratcher
Copy link
Member

HttpRequestMessage.RequestUri is what transforms can set to override DestinationPrefix.

// Allow a transform to directly set a custom RequestUri.
if (proxyRequest.RequestUri is null)
{
var queryString = transformContext.MaybeQuery?.QueryString ?? httpContext.Request.QueryString;
proxyRequest.RequestUri = RequestUtilities.MakeDestinationAddress(
transformContext.DestinationPrefix, transformContext.Path, queryString);
}

I guess we could make DestinationPrefix mutable since you can already override it via RequestUri. Careful though, changing the destination prefix at this point can mess up load balancing, health checks, etc. if the proxy is attributing state from one prefix to another host.

Why do you want to change the prefix?

@TrevorDArcyEvans
Copy link
Author

LOL! I am already overriding it via RequestUri and it is a bit fiddly. Updating DestinationPrefix is much cleaner and simpler, albeit with the riders you pointed out.

Could we make it mutable with some suitable warnings in the comments?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted We will welcome a contribution Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants