Input Validation

This learning is based on a real production incident that affected customers.

Let’s examine a real-world example to understand the impact of thorough code reviews, considering input validation.

The Scenario

A developer created a reconciliation report to get a list of transactions that were created after a given date. The intention was to apply the date-part of the filter, omitting the time. However, there was no input validation or testing to ensure that the filter was applied correctly. This couldn’t be caught before production deployment because:

There was no automated tests.
There was no input validation to ensure that the caller cannot pass a time-part in the filter.

Before

reconReport.ts


function getTransactionsAfterDate(createdAfter) {
  const query = `
    SELECT * FROM transactions 
    WHERE created_at >= '${createdAfter}'
  `;
 
  return db.executeQuery(query);
}

PR Comment

Choose the comment that you think is the most constructive and helpful.

Berte Goldcommented on Apr 2, 2025

Looks good to me, would be good be better to use concrete types instead of any, but approved as that can be improved later.

Anneliese Harlequincommented on Apr 2, 2025

The SQL looks good. We should also add some tests to verify that it returns the correct results.

Charlean Turquoisecommented on Apr 2, 2025

The SQL looks good. We should use concrete types instead of any to ensure that the parameter is indeed a date. Also please add some tests to verify that the filter is applied correctly.

Eilis Ivorycommented on Apr 2, 2025

Thanks for the implementing the recon report! The SQL looks good, but we should ensure that the inputs are validated and the logic works as expected.

We should use concrete types instead of any to ensure that the caller is passing a date object.
This method should also enforce the date format to strip the time part, in case the caller passes a datetime object.
We should add tests to verify that the filter is applied correctly, ensuring that the time part is not included in the filter.

Click here to learn more

Key Lessons

1. Promoting Input Validation

Always use concrete types as that provides type safety out of the box
Be transparent about the assumptions that are made about the input
Document the the requirements for the inputs, and make sure address all of them in the validation

2. Testing Strategy

Make sure to cover different input permutations in the tests
Think about non-standard input cases, like null, undefined, or empty strings
Incorporate both positive and negative cases in the automated tests

3. Code Review Best Practices

Reviewers should be mindful about input validation
Promote better code quality before merging the changes where possible
Suggest testing scenarios that are relevant to the changes and make sure they’re covered

Tips for Reviewers

1. Ask About Input Requirements

What is the functional requirements for this API?
What are the constraints on this particular argument?
Are we using concrete types for all inputs?
Example: “Is it expected for this method to return all the transactions occurred in a given date, ignoring the time part?“

2. Verify Testing Strategy

What are the testing scenarios we’re considering?
Have we covered the non-standard input cases?
Example: “Have we tested the method with different inputs like null, undefined, or empty strings?”
Example: “Have we tested the method with transactions that have different create timestamps in the db?”

Common Pitfalls to Avoid

1. Focusing Only on Business Logic

❌ “The code looks good, approved.”
✅ “The code looks good, but we should use concrete types for better type safety, it gives more confidence on input validation too.”

2. Comprehensive Testing Scenarios

❌ “We should also add some tests to verify we get correct results.”
✅ “We should add both happy path tests, and also negative tests to verify the code behaves as expected for non-standard input cases.”

3. Overlooking Input Validation

❌ “Looks good, inputs are incorporated correctly in the query.”
✅ “Inputs are set correctly in the query, but we should use also do input validation for all arguments, ensuring that the caller can only pass in expected values.”

Remember: A good code review always considers input validation, especially for externally exposed codepaths. Also promotes better code quality before merging the changes where possible.