fix: stop leaking optional query params and headers across subsequent… #51
Conversation
| @@ -263,17 +263,17 @@ export class BaseService { | |||
| @nonHTTPRequestMethod | |||
| private _resolveQuery(methodName: string, args: any[]): any { | |||
| const meta = this.__meta__; | |||
| const query = meta[methodName].query || {}; | |||
There was a problem hiding this comment.
Issue 1 - leaking across requests
If method has an @Queries decorator, this.__meta__[methodName].query exists and is pulled in by reference, so when appending new params to the query object these end up being persisted on the instance and so undesirably get pulled into the next request via this.__meta__[methodName].query
this.__meta__[methodName].query is only safe for and meant for static query params, cloning the query object before manipulation breaks the reference and fixes this issue
| const queryParams = meta[methodName].queryParams; | ||
| for (const pos in queryParams) { | ||
| if (queryParams[pos]) { | ||
| if (queryParams[pos] && args[pos] !== undefined) { |
There was a problem hiding this comment.
Issue 2 - sending undefined as value
Once leak was fixed, optional params with no value specified would always be present as keys in query config object with value of undefined...
{
myoptionalparam: undefined,
myrequiredparam: 'hello'
}
This isn't desirable as given the query string /something?myoptionalparam=undefined a server would interpret this as a string with value of "undefined"
For headers this effect was worse as errored due to trying to call setHeader(undefined) which is not allowed
Avoiding adding unspecified optional query and header args to config avoids this, so they don't get sent with the request at all
| query[queryParams[pos]] = args[pos]; | ||
| } | ||
| } | ||
| const queryMapIndex = meta[methodName].queryMapIndex; | ||
| if (queryMapIndex >= 0) { | ||
| for (const key in args[queryMapIndex]) { | ||
| if (args[queryMapIndex][key]) { | ||
| if (args[queryMapIndex][key] !== undefined) { |
There was a problem hiding this comment.
Issue 3 - discarding legitimate falsy values
While this bit technically was already preventing undefined values being added to the config object, what it actually does is filter out all falsy values. Therefore empty strings "", number 0, bolean false etc which can all be legitimate values would be discarded. Using an explicit test for undefined allows legit values to pass through
alechill
force-pushed
the
bugfix/optional-params-and-headers-leak-across-subsequent-requests
branch
2 times, most recently
from
9e9ac2c
to
892008d
Compare
alechill
force-pushed
the
bugfix/optional-params-and-headers-leak-across-subsequent-requests
branch
from
892008d
to
5214590
Compare
… requests
Fixes #50 and a couple of related minor issues
I can also see that the same issues may apply to the the @field and parts resolver, but have left those as is for now as i don't have a use case to test those against