microsoft / reverse-proxy

A toolkit for developing high-performance HTTP reverse proxy applications.
https://microsoft.github.io/reverse-proxy
MIT License
8.55k stars 838 forks source link

QueryParameterTransform should allow URL parameter with value that is empty string #619

Closed weichch closed 3 years ago

weichch commented 3 years ago

Describe the bug

I'm proxying request from http://source/path to http://destination/path?param1=&param2=value, with two transforms:

new QueryParameterFromStaticTransform(QueryStringTransformMode.Set,"param1","")
new QueryParameterFromStaticTransform(QueryStringTransformMode.Set,"param2","value")

However, param1 is dropped while being applied to RequestParametersTransformContext due to this line of code.

Empty string is valid variable value as per RFC 6570 and in my case the destination server cannot work without param1.

To Reproduce

var context = new RequestParametersTransformContext
{
    Query = new QueryTransformContext(HttpContext.Request)
};
new QueryParameterFromStaticTransform(QueryStringTransformMode.Set, "Empty", "").Apply(context);

// url should equal to ?Empty= but is empty string atm
var url = context.Query.QueryString.ToString();

Further technical details

Tratcher commented 3 years ago

That makes sense, but it's going to require some design changes. Most transforms use the return of an empty value to indicate the header, query param, etc, should be removed. You're right that an empty query param is valid, though I don't know how common it is to require its presence. We'll have to re-think how transforms indicate removal.

karelz commented 3 years ago

Triage: Distinguish between empty string and null? Given that it affects design, we should do it in 1.0.

weichch commented 3 years ago

@Tratcher I agree it's not commonly seen but unfortunately our destination server uses the key of the parameter instead of the value, and it doesn't allow any value set. I was thinking a new property AllowEmptyValue which defaults to false, then I can explicitly set it to true when needed. Similar to OpenAPI specs:

Empty-Valued and Nullable Parameters Query string parameters may only have a name and no value, like so: GET /foo?metadata Use allowEmptyValue to describe such parameters: