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

[core-client] query parameter overwritten with default value in absolute URLs #18264

Closed
deyaaeldeen opened this issue Oct 19, 2021 · 0 comments · Fixed by #18310
Closed

[core-client] query parameter overwritten with default value in absolute URLs #18264

deyaaeldeen opened this issue Oct 19, 2021 · 0 comments · Fixed by #18310
Assignees
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.

Comments

@deyaaeldeen
Copy link
Member

deyaaeldeen commented Oct 19, 2021

Consider a scenario where a response has a nextLink that is an absolute URL. This URL typically has query parameters that control which page to return next such as $top and $skip. Those values are important and should be preserved when a request is sent to that URL but what happens instead is that those query values get overwritten with default values if exist in the mapper. See

let queryParameterValue: string | string[] = getOperationArgumentValueFromParameter(
operationArguments,
queryParameter,
fallbackObject
);

Notice that it calculates the query parameter value regardless of whether that query already exists in the absolute URL or not.

Then, default query parameter values get combined with existing ones by putting them in a set:

for (const [name, value] of queryParams) {
const existingValue = combinedParams.get(name);
if (Array.isArray(existingValue)) {
if (Array.isArray(value)) {
existingValue.push(...value);
const valueSet = new Set(existingValue);
combinedParams.set(name, Array.from(valueSet));
} else {
existingValue.push(value);
}
} else if (existingValue) {
let newValue = value;
if (Array.isArray(value)) {
value.unshift(existingValue);
} else if (sequenceParams.has(name)) {
newValue = [existingValue, value];
}
combinedParams.set(name, newValue);
} else {
combinedParams.set(name, value);
}
}

Which effectively replace the old ones with new ones.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.
Projects
None yet
2 participants