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

Migrated all API interfaces from docs to shared #42

Merged
merged 5 commits into from
Aug 27, 2021

Conversation

chomosuke
Copy link
Owner

@chomosuke chomosuke commented Aug 27, 2021

From now on, use the API defined interfaces ensure the response type is correct as shangzhe has done in the me endpoint

@chomosuke chomosuke added the enhancement New feature or request label Aug 27, 2021
@chomosuke chomosuke added this to the Sprint 1 milestone Aug 27, 2021
@chomosuke chomosuke self-assigned this Aug 27, 2021
@chomosuke chomosuke added this to Sprint 1 in Product backlog via automation Aug 27, 2021
@chomosuke chomosuke added this to In progress in Sprint 1 via automation Aug 27, 2021
@chomosuke
Copy link
Owner Author

I don't know why does the test fail...

@shangzhel
Copy link
Collaborator

From now on, convert post body to the appropriate type first using as before deconstructing.

No, absolutely DO NOT DO THIS. Using as to cast types bypasses TypeScript's type checking facility. The reason req.body is any instead of whatever else is because body can contain anything, including nothing. Consider the following JSON request body:

{}

When this gets to a route handler, req.body will equal {}. If at this point you cast it to something using as, like currently

body as LoginRequest

You are telling the TypeScript compiler to assume that body contains a non-null username and password that are strings, when there may not be. Afterwards, code that runs will use this incorrect assumption and either crash or expose some security hole. This is a prime example of forgetting to validate user input.

For the same reason, do not ever cast user input. Always follow these steps when accessing user-provided objects:

  1. Check that object is not null (e.g. req.body != null)
    1. This avoids dereferencing null and undefined and generating exceptions
  2. Check each individual property that you want is present or otherwise (e.g. typeof req.body.username === 'string')
    1. This checks that input values are the exactly the right types that you want, so that if you do value + 1 you know whether to expect the result to be 2 if value === 1 or 'something1' if value === 'something'.
  3. Copy out exactly which properties you want and nothing more
    1. Malicious input can give itself properties that take the place of what you would normally assume is something else. A JSON string like {"toString":0} will replace the toString function with the number 0, making you crash when you try to call JSON.parse('{"toString":0}').toString(). Copying only the properties that you want makes sure nothing that you are unaware of gets overwritten.

@chomosuke
Copy link
Owner Author

okay how about this, now it's only migrating, the Request types are left in shared for future front end use

Copy link
Collaborator

@shangzhel shangzhel left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@chomosuke chomosuke merged commit ec10858 into master Aug 27, 2021
Sprint 1 automation moved this from In progress to Done Aug 27, 2021
Product backlog automation moved this from Sprint 1 to Sprint 1 Done Aug 27, 2021
@chomosuke chomosuke deleted the richard/docs-to-shared branch August 27, 2021 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Product backlog
  
Sprint 1 Done
Sprint 1
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants