Skip to content

Conversation

@bajtos
Copy link
Member

@bajtos bajtos commented Sep 24, 2018

Add a missing coercion step for string parameters and prevent the situation when a string value provided by the HTTP client and converted into an object or an array by query-string parser is passed as-is to the controller method, violating the endpoint contract and TypeScript declaration.

I discovered this problem while working on #1679. Without this change, endpoints like TodoListController#count are incorrectly accepting object values for their where parameter described as type: string.

https://github.com/strongloop/loopback-next/blob/d65a17fab7ce44b7977e9c03388f5684558475f2/examples/todo-list/src/controllers/todo-list.controller.ts#L37-L39

Checklist

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

@bajtos bajtos self-assigned this Sep 24, 2018
@bajtos bajtos requested review from a team, jannyHou and raymondfeng September 24, 2018 09:39
Copy link
Contributor

@jannyHou jannyHou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Good catch!
LGTM

});

describe('coerce param from string to string - optional', () => {
context('valid values', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: no need to add async I think

test(OPTIONAL_STRING_PARAM, undefined, undefined);
});

// @jannyhou For review: shall we convert empty value to 0 or throw error?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙈 the comment can be removed. thanks!

Add a missing coercion step for string parameters and prevent the
situation when a string value provided by the HTTP client and converted
into an object or an array by query-string parser is passed as-is
to the controller method, violating the endpoint contract and TypeScript
declaration.
@bajtos bajtos force-pushed the fix/string-coercion branch from cad1af6 to a67948f Compare September 25, 2018 07:41
@bajtos bajtos merged commit 1f49844 into master Sep 25, 2018
@bajtos bajtos deleted the fix/string-coercion branch September 25, 2018 08:01
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

Successfully merging this pull request may close these issues.

4 participants