Code Review: Image Upload

[komplexb]

⚠️ Issues & Concerns

Security & Best Practices:

1. Potential memory exhaustion (onenote.js:403-409):

   res.on('data', chunk => {
     downloadedBytes += chunk.length
     if (downloadedBytes > maxSizeBytes) {
       return callback(new Error(`Image download exceeded size limit: ${downloadedBytes} bytes`))
     }
     chunks.push(chunk)
   })

   The chunks array continues to grow even after size limit is exceeded. Should abort the request immediately.

2. Inconsistent timeout handling (onenote.js:356, notify.js:269):

   • getImageSize(): 10s response, 15s deadline
   • downloadImage(): 30s response, 45s deadline
   • sendPhotoToTelegram(): 30s response, 45s deadline

   Consider standardizing timeout values or making them configurable.

3. Missing input validation (onenote.js:327-343):
   extractFirstImage() doesn't validate that htmlContent is a string or handle null/undefined inputs.

Code Quality:

4. Recursive call potential (onenote.js:316):

   if (recentNotes.includes(note.id)) {
     return getRandomNote(notes, sectionHandle)
   }

   Could cause stack overflow if all notes are recent. Consider iterative approach or max retry limit.

5. Hardcoded filename (notify.js:268):

   .attach('photo', imageBuffer, 'image.jpg')

   Should derive filename from image URL or content-type header.

6. Unclear error propagation (notify.js:180-183):
   Image errors are logged as warnings but might mask important issues. Consider differentiating between expected vs unexpected errors.

🔧 Recommendations

1. Fix memory leak in downloadImage():

   res.on('data', chunk => {
     downloadedBytes += chunk.length
     if (downloadedBytes > maxSizeBytes) {
       res.destroy() // Abort the request
       return callback(new Error(`Image download exceeded size limit`))
     }
     chunks.push(chunk)
   })

2. Add input validation:

   function extractFirstImage(htmlContent) {
     if (!htmlContent || typeof htmlContent !== 'string') {
       return null
     }
     // ... rest of function
   }