Skip to content

sakshi-js-fetch-security-debugging-assignments #529

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.

Sign up for GitHub

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

Open
sakshibhosale4 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

sakshi-js-fetch-security-debugging-assignments #529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions javascript/10-fetch-security-debugging/sakshi/assignment-1/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>Search Products</title>
<link rel="stylesheet" href="./styles/styles.css">
</head>
<body>
<header>
<h1><a href="index.html">E-Shop</a></h1>
<nav>
<a href="index.html">Search</a>
<a href="list.html">All Products</a>
<a href="view.html">View Product</a>
</nav>
</header>

<main>
<h2>Search Products</h2>
<input type="text" id="searchInput" placeholder="Enter product name...">
<button onclick="searchProducts()">Search</button>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Oct 1, 2025

Choose a reason for hiding this comment

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

medium

It's a best practice to avoid inline event handlers like onclick. Instead, you should add event listeners in your JavaScript file. This helps to separate your HTML structure from your JavaScript logic, making the code cleaner and easier to maintain. You can add an id to the button to select it in your script.

In your script.js, you would then add something like this:

const searchButton = document.getElementById('searchButton');
if (searchButton) {
  searchButton.addEventListener('click', searchProducts);
}
Suggested change
<button onclick="searchProducts()">Search</button>
<button id="searchButton">Search</button>

<div id="results" class="grid"></div>
</main>

<script src="./scripts/script.js"></script>
</body>
</html>
26 changes: 26 additions & 0 deletions javascript/10-fetch-security-debugging/sakshi/assignment-1/list.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>All Products</title>
<link rel="stylesheet" href="./styles/styles.css">
</head>
<body>
<header>
<h1><a href="index.html">E-Shop</a></h1>
<nav>
<a href="index.html">Search</a>
<a href="list.html">All Products</a>
<a href="view.html">View Product</a>
</nav>
</header>

<main>
<h2>All Products</h2>
<button onclick="loadProducts()">Load Products</button>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Oct 1, 2025

Choose a reason for hiding this comment

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

medium

It's a best practice to avoid inline event handlers like onclick. Instead, you should add event listeners in your JavaScript file. This helps to separate your HTML structure from your JavaScript logic, making the code cleaner and easier to maintain. You can add an id to the button to select it in your script.

In your script.js, you would then add something like this:

const loadButton = document.getElementById('loadProductsButton');
if (loadButton) {
  loadButton.addEventListener('click', loadProducts);
}
Suggested change
<button onclick="loadProducts()">Load Products</button>
<button id="loadProductsButton">Load Products</button>

<div id="products" class="grid"></div>
</main>

<script src="./scripts/script.js"></script>
</body>
</html>
60 changes: 60 additions & 0 deletions javascript/10-fetch-security-debugging/sakshi/assignment-1/scripts/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
async function searchProducts() {
const query = document.getElementById("searchInput").value
const resultsDiv = document.getElementById("results")
resultsDiv.innerHTML = "Loading..."
try {
const res = await fetch(`https://dummyjson.com/products/search?q=${query}`)
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Oct 1, 2025

Choose a reason for hiding this comment

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

medium

When including user input in a URL, it's important to properly encode it to handle special characters (e.g., &, ?, /). While fetch does some encoding, it's best practice to be explicit by using encodeURIComponent to prevent potential request errors or injection vulnerabilities.

Suggested change
const res = await fetch(`https://dummyjson.com/products/search?q=${query}`)
const res = await fetch(`https://dummyjson.com/products/search?q=${encodeURIComponent(query)}`)

if (!res.ok) throw new Error("Failed to fetch products")
const data = await res.json()
resultsDiv.innerHTML = ""
data.products.forEach(p => {
resultsDiv.innerHTML += `
<div class="product-card">
<img src="${p.thumbnail}" alt="${p.title}">
<h3>${p.title}</h3>
<p>$${p.price}</p>
</div>`
})
Comment on lines +9 to +17
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Oct 1, 2025

Choose a reason for hiding this comment

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

critical

Using innerHTML += inside a loop to build the UI has two major issues:

  1. Security Risk (XSS): You are injecting raw data from the API (p.thumbnail, p.title, p.price) directly into the HTML. If the API response contains malicious scripts (e.g., in the product title), they will be executed in the user's browser. This is a critical Cross-Site Scripting (XSS) vulnerability.
  2. Performance: Modifying innerHTML in a loop forces the browser to re-parse and re-render the DOM on each iteration, which is very inefficient for large lists.

A much safer and more performant approach is to create DOM elements programmatically and set their content using textContent, which automatically sanitizes the input. You can build all elements and append them to the DOM in a single operation using a DocumentFragment.

Suggested change
resultsDiv.innerHTML = ""
data.products.forEach(p => {
resultsDiv.innerHTML += `
<div class="product-card">
<img src="${p.thumbnail}" alt="${p.title}">
<h3>${p.title}</h3>
<p>$${p.price}</p>
</div>`
})
resultsDiv.innerHTML = "";
const fragment = document.createDocumentFragment();
data.products.forEach(p => {
const card = document.createElement('div');
card.className = 'product-card';
const img = document.createElement('img');
img.src = p.thumbnail;
img.alt = p.title;
const title = document.createElement('h3');
title.textContent = p.title;
const price = document.createElement('p');
price.textContent = `$${p.price}`;
card.append(img, title, price);
fragment.appendChild(card);
});
resultsDiv.appendChild(fragment);

} catch (err) {
resultsDiv.innerHTML = `<p style="color:red">${err.message}</p>`
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Oct 1, 2025

Choose a reason for hiding this comment

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

high

Displaying raw error messages via innerHTML is risky. It can leak internal application details and create a Cross-Site Scripting (XSS) vulnerability if the error message contains malicious HTML. It's safer to show a generic, user-friendly message and use textContent to prevent script execution. Also, using inline styles is not a good practice; styles should be in your CSS file.

Suggested change
resultsDiv.innerHTML = `<p style="color:red">${err.message}</p>`
resultsDiv.textContent = `An error occurred while fetching products.`;

}
}

async function loadProducts() {
const productsDiv = document.getElementById("products")
productsDiv.innerHTML = "Loading..."
try {
const res = await fetch("https://dummyjson.com/products?limit=12")
if (!res.ok) throw new Error("Failed to fetch products")
const data = await res.json()
productsDiv.innerHTML = ""
data.products.forEach(p => {
productsDiv.innerHTML += `
<div class="product-card">
<img src="${p.thumbnail}" alt="${p.title}">
<h3>${p.title}</h3>
<p>$${p.price}</p>
</div>`
})
Comment on lines +30 to +38
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Oct 1, 2025

Choose a reason for hiding this comment

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

critical

Similar to searchProducts, using innerHTML += here introduces a critical Cross-Site Scripting (XSS) vulnerability and is inefficient. Data from the API should never be trusted and inserted directly into innerHTML. Please create DOM elements dynamically to prevent this. This logic is also duplicated from searchProducts; consider creating a helper function to generate product cards to follow the DRY principle.

Suggested change
productsDiv.innerHTML = ""
data.products.forEach(p => {
productsDiv.innerHTML += `
<div class="product-card">
<img src="${p.thumbnail}" alt="${p.title}">
<h3>${p.title}</h3>
<p>$${p.price}</p>
</div>`
})
productsDiv.innerHTML = "";
const fragment = document.createDocumentFragment();
data.products.forEach(p => {
const card = document.createElement('div');
card.className = 'product-card';
const img = document.createElement('img');
img.src = p.thumbnail;
img.alt = p.title;
const title = document.createElement('h3');
title.textContent = p.title;
const price = document.createElement('p');
price.textContent = `$${p.price}`;
card.append(img, title, price);
fragment.appendChild(card);
});
productsDiv.appendChild(fragment);

} catch (err) {
productsDiv.innerHTML = `<p style="color:red">${err.message}</p>`
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Oct 1, 2025

Choose a reason for hiding this comment

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

high

Displaying raw error messages via innerHTML is risky. It can leak internal application details and create a Cross-Site Scripting (XSS) vulnerability if the error message contains malicious HTML. It's safer to show a generic, user-friendly message and use textContent to prevent script execution. Also, using inline styles is not a good practice; styles should be in your CSS file.

Suggested change
productsDiv.innerHTML = `<p style="color:red">${err.message}</p>`
productsDiv.textContent = `An error occurred while fetching products.`;

}
}

async function viewProduct() {
const id = document.getElementById("productId").value
const detailsDiv = document.getElementById("productDetails")
detailsDiv.innerHTML = "Loading..."
try {
const res = await fetch(`https://dummyjson.com/products/${id}`)
if (!res.ok) throw new Error("Product not found")
const p = await res.json()
detailsDiv.innerHTML = `
<img src="${p.thumbnail}" alt="${p.title}">
<h3>${p.title}</h3>
<p><strong>Price:</strong> $${p.price}</p>
<p>${p.description}</p>`
Comment on lines +52 to +56
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Oct 1, 2025

Choose a reason for hiding this comment

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

critical

Directly setting innerHTML with API response data (p.thumbnail, p.title, p.description, etc.) is a major security risk, as it can lead to Cross-Site Scripting (XSS) attacks. Any malicious script in the product data would be executed. You should create DOM elements and use textContent to safely display the data.

    detailsDivinnerHTML = '';

    const img = document.createElement('img');
    img.src = p.thumbnail;
    img.alt = p.title;

    const title = document.createElement('h3');
    title.textContent = p.title;

    const price = document.createElement('p');
    const priceLabel = document.createElement('strong');
    priceLabel.textContent = 'Price:';
    price.append(priceLabel, ` $${p.price}`);

    const description = document.createElement('p');
    description.textContent = p.description;

    detailsDiv.append(img, title, price, description);

} catch (err) {
detailsDiv.innerHTML = `<p style="color:red">${err.message}</p>`
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Oct 1, 2025

Choose a reason for hiding this comment

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

high

Displaying raw error messages via innerHTML is risky. It can leak internal application details and create a Cross-Site Scripting (XSS) vulnerability if the error message contains malicious HTML. It's safer to show a generic, user-friendly message and use textContent to prevent script execution. Also, using inline styles is not a good practice; styles should be in your CSS file.

Suggested change
detailsDiv.innerHTML = `<p style="color:red">${err.message}</p>`
detailsDiv.textContent = `An error occurred while fetching the product details.`;

}
}
82 changes: 82 additions & 0 deletions javascript/10-fetch-security-debugging/sakshi/assignment-1/styles/styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
body {
font-family: Arial, sans-serif;
margin: 0;
background: #f5f5f5;
color: #333;
}

header {
background: #2c3e50;
color: white;
padding: 1rem 2rem;
display: flex;
justify-content: space-between;
align-items: center;
}

header h1 a {
margin: 0;
font-size: 1.8rem;
color: white;

}

nav a {
color: white;
margin-left: 1rem;
text-decoration: none;
font-weight: bold;
transition: color 0.3s;
}

nav a:hover {
color: #f39c12;
}

main {
padding: 2rem;
}

input, button {
padding: 0.6rem;
margin: 0.5rem 0;
border-radius: 5px;
border: 1px solid #ccc;
}

button {
background: #3498db;
color: white;
border: none;
cursor: pointer;
}

button:hover {
background: #2980b9;
}

.grid {
display: grid;
grid-template-columns: repeat(auto-fit, minmax(220px, 1fr));
gap: 1.5rem;
margin-top: 1.5rem;
}

.product-card {
background: white;
border-radius: 10px;
padding: 1rem;
box-shadow: 0 4px 8px rgba(0,0,0,0.1);
text-align: center;
transition: transform 0.2s;
}

.product-card:hover {
transform: scale(1.05);
}

.product-card img {
max-width: 100%;
border-radius: 8px;
margin-bottom: 0.5rem;
}
27 changes: 27 additions & 0 deletions javascript/10-fetch-security-debugging/sakshi/assignment-1/view.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>View Product</title>
<link rel="stylesheet" href="./styles/styles.css">
</head>
<body>
<header>
<h1><a href="index.html">E-Shop</a></h1>
<nav>
<a href="index.html">Search</a>
<a href="list.html">All Products</a>
<a href="view.html">View Product</a>
</nav>
</header>

<main>
<h2>View Product by ID</h2>
<input type="number" id="productId" placeholder="Enter product ID">
<button onclick="viewProduct()">View Product</button>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Oct 1, 2025

Choose a reason for hiding this comment

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

medium

It's a best practice to avoid inline event handlers like onclick. Instead, you should add event listeners in your JavaScript file. This helps to separate your HTML structure from your JavaScript logic, making the code cleaner and easier to maintain. You can add an id to the button to select it in your script.

In your script.js, you would then add something like this:

const viewButton = document.getElementById('viewProductButton');
if (viewButton) {
  viewButton.addEventListener('click', viewProduct);
}
Suggested change
<button onclick="viewProduct()">View Product</button>
<button id="viewProductButton">View Product</button>

<div id="productDetails" class="product-card"></div>
</main>

<script src="./scripts/script.js"></script>
</body>
</html>
15 changes: 15 additions & 0 deletions javascript/10-fetch-security-debugging/sakshi/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>HTML Assignments | Sakshi Bhosale</title>
</head>
<body>
<details>
<summary>Assignment 1</summary>
<iframe src="./assignment-1/index.html" frameborder="0" width="100%"></iframe>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Oct 1, 2025

Choose a reason for hiding this comment

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

medium

The frameborder attribute is obsolete in HTML5 and should not be used. Instead, you should use CSS to control the iframe's border. You can apply border: none; via a style attribute or a CSS class.

Suggested change
<iframe src="./assignment-1/index.html" frameborder="0" width="100%"></iframe>
<iframe src="./assignment-1/index.html" width="100%" style="border: none;"></iframe>

</details>
</body>
</html>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Oct 1, 2025

Choose a reason for hiding this comment

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

medium

It's a standard convention to have a single newline character at the end of every text file. This file is missing one, which can cause issues with some tools and version control systems.