AOSSIE-Org / AOSSIE-Org/EduAid

[Bug/Refactor] Fix Unhandled Promises, Weak Cryptography (Math.random), and Code Quality Issues

Đang mở
#535 1 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
JavaScript
Star
171
Fork
425
Chỉ số merge pull request
Không có pull request nào được merge trong 30 ngày

Mô tả

## Description
During a recent static code analysis of the `eduaid_web` codebase (using SonarQube and ESLint), several bugs, security warnings, and code quality issues were identified. Addressing these will prevent silent runtime failures, improve security practices, and clean up the production build.

To keep things actionable, I have grouped the top findings into three main categories:

### 1. Unhandled Promise Rejections (Reliability - High)
Promises without proper error handling can cause silent failures and unhandled rejection errors in the browser.
* **`src/pages/Home.jsx` (Line 34):** The `fetchGitHubStars()` function intentionally throws an error if the network request fails (`if (!response.ok) throw new Error(...)`), but the `.then()` chain calling it lacks a `.catch()` block. If the API rate-limits the user, it will cause an unhandled promise rejection.
* **`src/reportWebVitals.js` (Line 3):** The dynamic import `import('web-vitals').then(...)` lacks a `.catch()` block to handle import failures gracefully.

### 2. Weak Cryptography / Insecure Randomness (Security - Medium)
The application relies on `Math.random()` for shuffling arrays. While this may not be a critical vulnerability for standard UI elements, using `Math.random()` is flagged as a Cryptographic Failure (CWE-338) and can lead to highly predictable token/shuffle generation.
* **`src/pages/Output.jsx` (Line 35):** `const j = Math.floor(Math.random() * (i + 1));`
* **`src/workers/pdfWorker.js` (Line 5):** `const j = Math.floor(Math.random() * (i + 1));`

### 3. Leftover Production Console Logs & Dead Code (Maintainability - Low)
* **Production Console Logs:** Multiple instances of `console.error` are left in production components (`src/pages/Text_Input.jsx`, `src/pages/Output.jsx`, `src/utils/apiClient.js`). These should ideally be stripped out or handled via a proper logging utility in production environments.
* **Unused Variables:** There are dozens of unused variables declared across the app (e.g., `stars` state in `Home.jsx`, unused DOM event parameters). These consume memory and trigger unnecessary linter warnings.

## Expected Behavior
* Failed `fetch` requests and dynamic imports should be properly caught and handled (e.g., logging the error safely or displaying a fallback UI state).
* Array shuffling logic should ideally use the cryptographically secure `window.crypto.getRandomValues()` API instead of `Math.random()`.
* Production builds should be completely free of dev-time `console` statements and unused/dead code.

## Proposed Solution
1. **Promises:** Append `.catch((error) => { ... })` to the affected promises in `Home.jsx` and `reportWebVitals.js`.
2. **Security:** Replace `Math.random()` in the `shuffleArray` utility functions with a secure implementation. For example:
```javascript
const randomBuffer = new Uint32Array(1);
window.crypto.getRandomValues(randomBuffer);
const randomFraction = randomBuffer[0] / (0xffffffff + 1);
const j = Math.floor(randomFraction * (i + 1));

Hướng dẫn đóng góp

Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.