feature/us-04-private-test-cases#15
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6835b1f472
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| }); | ||
|
|
||
| router.get('/:problemId', async (req, res, next) => { |
There was a problem hiding this comment.
This route is mounted directly as GET /test-cases/:problemId without any middleware, so any API client can fetch the full payload returned by getAllTestCases, including cases where is_sample is false. The commit describes this endpoint as intended for the internal judge, while the existing repo protects authenticated routes with authGuard in src/modules/auth/auth.controller.js; without an internal credential or similar guard here, students can retrieve hidden inputs and expected outputs and bypass the private-test mechanism.
Useful? React with 👍 / 👎.
| const notFoundError = new Error('Test cases file not found'); | ||
| notFoundError.statusCode = 404; | ||
| throw notFoundError; |
There was a problem hiding this comment.
Setting notFoundError.statusCode = 404 here does not affect the HTTP response because the app-level error middleware in src/main.js ignores error.statusCode and maps everything except two Google auth messages to 500. As a result, GET /test-cases/<missing problem> reports an internal server error instead of a normal not-found condition, which will mislead clients and monitoring for absent problem files unless the controller returns 404 directly or the shared error handler honors this status.
Useful? React with 👍 / 👎.
| const TEST_CASES_DIR = path.resolve(__dirname, '../../files/test-cases'); | ||
|
|
||
| async function getAllTestCases(problemId) { | ||
| const filePath = path.join(TEST_CASES_DIR, `problema-${problemId}.json`); |
There was a problem hiding this comment.
Because Express decodes route parameters, a request can supply an encoded slash in problemId (for example %2F..%2F) and this path.join will normalize the resulting ../ segments before reading the file. In that case the endpoint is no longer confined to src/files/test-cases, so it can disclose any readable JSON array file whose normalized name fits the problema-*.json pattern, or at least expose file-existence/format differences through the error messages; restrict IDs to the expected numeric form or verify the resolved path stays under TEST_CASES_DIR before reading.
Useful? React with 👍 / 👎.
|
|
||
| const allCases = await testCasesService.getAllTestCases('1', tempDir); |
There was a problem hiding this comment.
These calls pass tempDir, but getAllTestCases and getPublicTestCases only accept problemId, so the extra argument is ignored and the test reads the committed src/files/test-cases/problema-1.json fixture instead of the file created above. This lets the test keep passing even if the service is not reading the intended fixture contents, and it also means the claimed ordering/separation behavior is only checked against whatever happens to be in the production sample file.
Useful? React with 👍 / 👎.
| throw notFoundError; | ||
| } | ||
|
|
||
| if (error.message === 'Unexpected token') { |
There was a problem hiding this comment.
For a malformed test-case file, JSON.parse does not throw an error whose message is exactly Unexpected token (it includes the token/position or other details), so this branch is skipped and clients receive the raw parser message instead of the intended generic Invalid JSON in test cases file. If a fixture is corrupted, especially one containing hidden cases, matching on SyntaxError (or wrapping only the parse call) avoids exposing parser snippets and keeps the error contract consistent.
Useful? React with 👍 / 👎.
2 participants
Se implementa el módulo de test cases para gestionar casos de prueba desde un file server.
Se crea el módulo
src/modules/test-cases/con dos endpoints GET:GET /test-cases/:problemIdretorna todos los casos, para el juez interno.GET /test-cases/public/:problemIdretorna solo casos públicos, para el usuario.El servicio lee archivos JSON desde
src/files/test-cases/, separa casos públicos (is_sample=true) de privados (is_sample=false) y mantiene el orden mediantesort_order.Se añadieron pruebas básicas que validan la separación público/privado y el orden correcto.
Se agregan 3 ejemplos en
src/files/test-cases/.