-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add to cart #2
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR implements a shopping cart feature for an e-commerce application built with Symfony. It adds functionality to view products, add them to a cart, manage cart contents, and navigate between products and cart views.
Changes
- Added Cart entity and CartRepository for cart data management
- Created CartController with methods for viewing, adding, removing, and clearing cart items
- Added product detail view functionality to ProductController
- Created templates for cart view and product detail pages
- Updated product listing template to include cart integration
- Added README documentation for the application features
Impact
- Enables users to browse products and add them to a shopping cart
- Provides session-based cart persistence
- Allows users to manage cart contents (update quantities, remove items)
- Creates a foundation for future checkout functionality
| public function findCartBySessionId($sessionId) | ||
| { | ||
| $conn = $this->entityManager->getConnection(); | ||
| $sql = 'SELECT * FROM cart WHERE session_id = "' . $sessionId . '"'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
security (critical): SQL Injection vulnerability in the findCartBySessionId method. The sessionId parameter is directly concatenated into the SQL query without proper escaping or parameterization.
This allows attackers to inject malicious SQL code by manipulating the session ID, potentially leading to unauthorized data access or modification.
Fix by using parameterized queries:
$sql = 'SELECT * FROM cart WHERE session_id = :sessionId';
$stmt = $conn->prepare($sql);
$stmt->bindValue('sessionId', $sessionId);
$resultSet = $stmt->executeQuery();Alternatively, use Doctrine's query builder:
return $this->createQueryBuilder('c')
->where('c.sessionId = :sessionId')
->setParameter('sessionId', $sessionId)
->getQuery()
->getResult();Help us improve our suggestions - react with 👍 if it was helpful, 👎 if it needs work
| { | ||
| $sessionId = $this->session->getId(); | ||
|
|
||
| $cart = $this->cartRepository->findOneBy(['sessionId' => $sessionId]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bug (high): Inconsistency between CartController.removeFromCart() and CartController.viewCart(). The removeFromCart method uses findOneBy() which returns a Cart entity, but viewCart uses findCartBySessionId() which returns an array of data.
This inconsistency will cause errors when users try to remove items from their cart because the code expects different data structures in different methods.
Fix by using consistent repository methods:
public function removeFromCart($id)
{
$sessionId = $this->session->getId();
$cartData = $this->cartRepository->findCartBySessionId($sessionId);
if (!empty($cartData)) {
$cart = $cartData[0];
$items = json_decode($cart['items'], true);
unset($items[$id]);
// Update the cart with modified items
$this->cartRepository->saveCart($sessionId, $items);
}
return $this->redirectToRoute('cart_view');
}Help us improve our suggestions - react with 👍 if it was helpful, 👎 if it needs work
| return $this->redirectToRoute('product_list'); | ||
| } | ||
|
|
||
| $quantity = $request->query->get('quantity', 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
security (medium): The addToCart method retrieves the quantity from query parameters but doesn't properly validate it. While there is a check for numeric values, it doesn't enforce positive integers or handle potential overflow issues.
Fix by adding proper validation:
$quantity = $request->query->get('quantity', 1);
// Ensure quantity is a positive integer
if (!is_numeric($quantity) || $quantity < 1) {
$quantity = 1;
} else {
// Convert to integer and cap at a reasonable maximum
$quantity = min((int)$quantity, 100); // Set appropriate maximum
}Help us improve our suggestions - react with 👍 if it was helpful, 👎 if it needs work
| <td>{{ item.product.name }}</td> | ||
| <td>${{ item.product.price }}</td> | ||
| <td> | ||
| <form action="/cart/update/{{ item.product.id }}" method="get" style="display: inline;"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bug (high): The template references a route '/cart/update/{id}' that doesn't exist in the CartController. This will result in a 404 error when users try to update quantities in their cart.
Fix by either:
- Adding the missing route in CartController:
/**
* @Route("/cart/update/{id}", name="cart_update")
*/
public function updateCartItem($id, Request $request)
{
$quantity = $request->query->get('quantity', 1);
$sessionId = $this->session->getId();
$this->cartRepository->addItemToCart($sessionId, $id, $quantity);
return $this->redirectToRoute('cart_view');
}- Or modifying the form to use an existing route like '/cart/add/{id}'
Help us improve our suggestions - react with 👍 if it was helpful, 👎 if it needs work
|
|
||
| <div class="row mt-4"> | ||
| <div class="col-md-6"> | ||
| <!-- Bad practice: GET request for destructive operation --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
security (medium): Using a GET request for the destructive operation of clearing the cart. This violates HTTP semantics (GET should be idempotent) and makes the application vulnerable to CSRF attacks and accidental operations through browser prefetching.
Fix by using a POST form with CSRF protection:
<form action="{{ path('cart_clear') }}" method="post" style="display: inline;">
<input type="hidden" name="_token" value="{{ csrf_token('clear-cart') }}">
<button type="submit" class="btn btn-warning">Clear Cart</button>
</form>And update the controller to check the CSRF token:
/**
* @Route("/cart/clear", name="cart_clear", methods={"POST"})
*/
public function clearCart(Request $request)
{
if (!$this->isCsrfTokenValid('clear-cart', $request->request->get('_token'))) {
throw new InvalidCsrfTokenException('Invalid CSRF token');
}
// Rest of the method...
}Help us improve our suggestions - react with 👍 if it was helpful, 👎 if it needs work
No description provided.