[BUGFIX] Introduce null dereference in findUserById#1
Open
GAURAV-1313 wants to merge 2 commits into
Open
Conversation
✅ Deploy Preview for nofriction canceled.
|
Owner
Author
AI Code ReviewSummaryThe PR drastically reduces utility functions by removing several functions and modifies findUserById to return an uppercased user name. However, the modified function contains multiple issues including unsafe null dereference and missing input validation, causing potential crashes and incorrect behavior on invalid inputs. Key FindingsBugs
Security
Performance
Recommended FixesGeneral
Security Fixes
Performance Fixes
Suggested Tests
const { findUserById } = require("./utils");
describe("findUserById", () => {
let users;
beforeEach(() => {
users = [
{ id: 1, name: "alice" },
{ id: 2, name: "Bob" },
{ id: 3, name: "charlie" }
];
});
it("should return the uppercase name of the user when found (happy path)", () => {
expect(findUserById(users, 2)).toBe("BOB");
expect(findUserById(users, 1)).toBe("ALICE");
});
it("should return null when no user matches the id", () => {
expect(findUserById(users, 999)).toBeNull();
expect(findUserById(users, -1)).toBeNull();
});
it("should handle empty users array input", () => {
expect(findUserById([], 1)).toBeNull();
});
it("should return correct uppercase name for single-element array", () => {
expect(findUserById([{ id: 1, name: "dave" }], 1)).toBe("DAVE");
});
it("should not mutate the input users array", () => {
const usersCopy = JSON.parse(JSON.stringify(users));
findUserById(users, 2);
expect(users).toEqual(usersCopy);
});
it.each([
[null, null],
[undefined, undefined],
[{ id: 1, name: null }, 1],
[{ id: 1, name: undefined }, 1]
])("should handle edge cases with null or undefined inputs", (customUsers, id) => {
if (customUsers) {
expect(findUserById(Array.isArray(customUsers) ? customUsers : [customUsers], id)).toBe(
customUsers[0] && customUsers[0].name ? customUsers[0].name.toUpperCase() : null
);
} else {
expect(findUserById(users, id)).toBeNull();
}
});
it("should handle numeric and string ids equivalently", () => {
expect(findUserById(users, "1")).toBe("ALICE");
});
});Auto-Fix AvailableAdd the 6 issues found. Reviewed by RepoSpace AI. |
- src/utils.js: Replaced inefficient sorting algorithm with Array.prototype.sort and improved date formatting.
Owner
Author
Auto-Fix ResultsApplied (1)
Generated by RepoSpace AI. Review AI commits before merging. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a null dereference bug by calling toUpperCase() on potentially null result.