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

'in' operator is less robust than other options for deriving key presence #440

Open
Packmanager9 opened this issue Mar 18, 2021 · 4 comments
Assignees

Comments

@Packmanager9
Copy link

Problem:
Given that the 'in' operator will throw an error error and fail: 'Cannot use 'in' operator to search for 'top' in 1'
(Line 4959 in 'msRest.node.js)

Solution
A different method of deriving the key presence could be used.

Original:

  if (parent != undefined && parameterPathPart in parent) {
      parent = parent[parameterPathPart];
  }

Proposed:

  if (parent != undefined && Object.keys(parent).includes(parameterPathPart)) {
      parent = parent[parameterPathPart];
  } 

Alternatives:
Tried removing the check all together, immediately fails.

@jeremymeng
Copy link
Member

@Packmanager9 The current code assumes that parent is an object, but the error indicates that it could be a primitive type. Do you have some simplified repro?

function getPropertyFromParameterPath(
parent: { [parameterName: string]: any },
parameterPath: string[]
): PropertySearchResult {
const result: PropertySearchResult = { propertyFound: false };
let i = 0;
for (; i < parameterPath.length; ++i) {
const parameterPathPart: string = parameterPath[i];
// Make sure to check inherited properties too, so don't use hasOwnProperty().
if (parent != undefined && parameterPathPart in parent) {
parent = parent[parameterPathPart];

The only call to this method is on a serviceClient which should be an object.

propertySearchResult = getPropertyFromParameterPath(serviceClient, parameterPath);
}

@Packmanager9
Copy link
Author

Packmanager9 commented Mar 22, 2021

Sure, here's the snippet I was using.

msRestNodeAuth.interactiveLogin().then((creds) => {
const client = new LogicManagementClient(creds, subscriptionId);
const top = 1;
const filter = "testfilter";
client.workflows.listBySubscription(top, filter).then((result) => {
console.log("The result is:");
console.log(result);
});
}).catch((err) => {
console.error(err);
});

@jeremymeng
Copy link
Member

@Packmanager9 could you please try wrapping the arguments to listBySubscription with an object?

-client.workflows.listBySubscription(top, filter)
+client.workflows.listBySubscription({top, filter})

The method expects an options object

  listBySubscription(options?: Models.WorkflowsListBySubscriptionOptionalParams): Promise<Models.WorkflowsListBySubscriptionResponse>;

@jeremymeng
Copy link
Member

@Packmanager9 it's been a while, have you got a chance to look at the above suggestion?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants