Skip to content

feat: increment opened count when opening link from local server (#255) #381

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

Merged
yogeshpaliyal merged 4 commits into from
Mar 8, 2026
Merged

feat: increment opened count when opening link from local server (#255) #381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5bafcbe
feat: increment opened count when opening link from local server (#255)
ALE-ARME Feb 8, 2026
05188c8
Merge branch 'master' into fix-issue-255
yogeshpaliyal Feb 21, 2026
e36b367
Merge branch 'master' into fix-issue-255
yogeshpaliyal Feb 21, 2026
2170bbd
Merge branch 'master' into fix-issue-255
yogeshpaliyal Mar 7, 2026
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
21 changes: 18 additions & 3 deletions app/src/main/assets/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -677,14 +677,14 @@ <h3 class="text-lg font-semibold text-gray-800 line-clamp-2 flex-1">
` : ''}

<div class="mt-4 pt-4 border-t border-gray-100">
<a href="${escapeHtml(link.link)}"
target="_blank"
<button
onclick="handleOpenLink(${link.id}, '${escapeHtml(link.link)}')"
class="w-full bg-gradient-to-r from-primary-500 to-secondary-500 hover:from-primary-600 hover:to-secondary-600 text-white py-2 px-4 rounded-lg text-sm font-medium transition-all duration-200 transform hover:scale-105 flex items-center justify-center gap-2">
<svg class="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24">
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M10 6H6a2 2 0 00-2 2v10a2 2 0 002 2h10a2 2 0 002-2v-4M14 4h6m0 0v6m0-6L10 14"></path>
</svg>
Open Link
</a>
</button>
Comment on lines +680 to +687
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 21, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Stored XSS via single-quote breakout in onclick attribute.

The escapeHtml() function (Line 890) encodes <, >, &, and " but does not escape single quotes ('). Since the URL is interpolated inside single-quoted string delimiters in the onclick attribute:

onclick="handleOpenLink(${link.id}, '${escapeHtml(link.link)}')"

A malicious URL containing a single quote (e.g., https://x.com/');alert(document.cookie);//) will break out of the string literal and execute arbitrary JavaScript when the card is rendered. This is a stored XSS — any user who can add a link via the API can inject script that runs for every viewer.

🔒 Proposed fix: use a data attribute and attach the handler programmatically, or escape single quotes

Option 1 (minimal fix): Escape single quotes in the interpolation context:

-                                    <button
-                                       onclick="handleOpenLink(${link.id}, '${escapeHtml(link.link)}')"
+                                    <button
+                                       onclick="handleOpenLink(${link.id}, '${escapeHtml(link.link).replace(/'/g, '&#39;')}')"

Option 2 (preferred): Use data-* attributes and bind handlers via addEventListener to avoid inline JS entirely. Store the id and url in data-link-id and data-link-url attributes (which are HTML-escaped by escapeHtml), then in DOMContentLoaded use event delegation:

document.getElementById('linksContainer').addEventListener('click', function(e) {
    const btn = e.target.closest('[data-link-id]');
    if (btn) {
        handleOpenLink(parseInt(btn.dataset.linkId, 10), btn.dataset.linkUrl);
    }
});

This avoids any inline JS injection surface.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/src/main/assets/index.html` around lines 680 - 687, The onclick
interpolation allows single-quote breakout because escapeHtml does not escape
single quotes; remove the inline onclick on the button and instead add data
attributes (e.g., data-link-id and data-link-url using escapeHtml for the URL)
and then attach a delegated click handler in DOMContentLoaded that locates the
clicked element (or closest button) and calls
handleOpenLink(parseInt(btn.dataset.linkId,10), btn.dataset.linkUrl);
alternatively, if you must keep inline JS, ensure escapeHtml is extended to
escape single quotes before interpolating into the single-quoted onclick, but
preferred is the data-* + addEventListener approach to eliminate inline JS
entirely.

</div>
</div>
</div>
Expand All @@ -693,6 +693,21 @@ <h3 class="text-lg font-semibold text-gray-800 line-clamp-2 flex-1">
`;
}

// Handle opening a link and incrementing count
async function handleOpenLink(id, url) {
// Open link in new tab
window.open(url, '_blank');

// Increment count in background
try {
await fetch(`/api/links/increment-count?id=${id}`, { method: 'POST' });
// Optional: refresh links to see updated count
setTimeout(loadLinks, 1000);
} catch (error) {
console.error('Error incrementing count:', error);
}
}

// Populate tag filter dropdown
function populateTagFilter() {
const tagFilter = document.getElementById('tagFilter');
Expand Down
18 changes: 18 additions & 0 deletions app/src/main/java/com/yogeshpaliyal/deepr/server/LocalServerRepositoryImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,24 @@ open class LocalServerRepositoryImpl(
)
}
}

post("/api/links/increment-count") {
try {
val id = call.request.queryParameters["id"]?.toLongOrNull()
if (id != null) {
accountViewModel.incrementOpenedCount(id)
call.respond(HttpStatusCode.OK, SuccessResponse("Count incremented"))
} else {
call.respond(HttpStatusCode.BadRequest, ErrorResponse("Invalid link ID"))
}
} catch (e: Exception) {
Log.e("LocalServer", "Error incrementing count", e)
call.respond(
HttpStatusCode.InternalServerError,
ErrorResponse("Error incrementing count: ${e.message}"),
)
}
}
Comment on lines +331 to +348
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 21, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Find AccountViewModel file
fd -t f "AccountViewModel" --type kotlin

Repository: yogeshpaliyal/Deepr

Length of output: 236

🏁 Script executed:

# Also try broader search in case naming differs
fd -t f -i "viewmodel" | grep -i account

Repository: yogeshpaliyal/Deepr

Length of output: 134

🏁 Script executed:

# Search for incrementOpenedCount in the entire codebase
rg -n "incrementOpenedCount" --type kotlin

Repository: yogeshpaliyal/Deepr

Length of output: 966

🏁 Script executed:

# Check the AccountViewModel.incrementOpenedCount implementation
cat -n app/src/main/java/com/yogeshpaliyal/deepr/viewmodel/AccountViewModel.kt | sed -n '488,495p'

Repository: yogeshpaliyal/Deepr

Length of output: 363

🏁 Script executed:

# Check the LinkRepositoryImpl.incrementOpenedCount implementation
cat -n app/src/main/java/com/yogeshpaliyal/deepr/data/LinkRepositoryImpl.kt | sed -n '219,235p'

Repository: yogeshpaliyal/Deepr

Length of output: 674

🏁 Script executed:

# Also check the LinkRepository interface to understand the contract
cat -n app/src/main/java/com/yogeshpaliyal/deepr/data/LinkRepository.kt | sed -n '107,110p'

Repository: yogeshpaliyal/Deepr

Length of output: 182

🏁 Script executed:

# Check how deeprQueries is used - likely SQLDelight generated code
rg -n "deeprQueries.incrementOpenedCount" --type-add 'kotlin:*.kt' -A 3 -B 3

Repository: yogeshpaliyal/Deepr

Length of output: 737

🏁 Script executed:

# Check if there's any logging or observable that might show race condition
rg -n "insertDeeprOpenLog" --type-add 'kotlin:*.kt' -A 2 -B 2

Repository: yogeshpaliyal/Deepr

Length of output: 2068

Critical race condition: endpoint returns 200 OK before database operation completes.

The issue is more severe than initial threading concern. accountViewModel.incrementOpenedCount(id) uses viewModelScope.launch(Dispatchers.IO) — a fire-and-forget coroutine that returns immediately without waiting for completion. The endpoint returns 200 OK to the client before the database increment and log operations have even started, creating a race condition and violating HTTP semantics.

The endpoint should either await the database operation (wrap in a suspend function and use a proper coroutine job) or use a blocking call if this is a local-only server.

Secondary issue: No validation that the given id corresponds to an existing link. Consider returning 404 if the ID doesn't exist, or ensure the implementation handles non-existent IDs gracefully.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/com/yogeshpaliyal/deepr/server/LocalServerRepositoryImpl.kt`
around lines 331 - 348, The POST handler currently calls
accountViewModel.incrementOpenedCount(id) which internally does
viewModelScope.launch(Dispatchers.IO) (fire-and-forget), so the route responds
200 before DB work completes; change the approach so the route awaits
completion: add or call a suspend function that performs the DB update (e.g.,
create incrementOpenedCountSuspend or expose a suspend DAO method used by
incrementOpenedCount) and invoke it with call.application.coroutineScope or
using runCatching { withContext(Dispatchers.IO) { /* suspend update */ } } to
await completion before responding; additionally check that the link exists (use
the DAO's find/get method) and return 404 if not found, or map the update result
to 200 vs 404 accordingly instead of unconditionally returning OK.

}
}

Expand Down
Loading
⚡