From 8f6b1b9b14ec7f2394f7bfb4960b92b705a95f5a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Nov 2025 17:06:18 +0000 Subject: [PATCH 1/3] Fix multiple code quality and security issues This commit addresses several critical and high-severity issues identified during code review: Security Fixes: - Disable cleartext HTTP traffic globally in network_security_config.xml - Remove API key logging (even partial) from ZoteroApiClient - Add null safety checks for API response fields to prevent NPEs Resource Management: - Fix resource leak in writeResponseBodyToDisk() using try-with-resources - Properly close streams even if exceptions occur during file operations Fragment Lifecycle: - Fix race conditions in CollectionFragment where getActivity() could become null between check and use - Store activity reference in local variable before checking and using it - Prevents crashes when fragments are detached during async operations Build Configuration: - Enable lint checks (abortOnError and checkReleaseBuilds) - Remove duplicate testOptions blocks - Allow lint to catch issues before they reach production --- app/build.gradle | 15 +---- .../oyvindbs/zotshelf/CollectionFragment.java | 55 +++++++++++-------- .../oyvindbs/zotshelf/ZoteroApiClient.java | 40 +++++++------- .../main/res/xml/network_security_config.xml | 4 +- 4 files changed, 56 insertions(+), 58 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 84f7aab..3b7f25d 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -5,11 +5,6 @@ plugins { android { namespace 'oyvindbs.zotshelf' - testOptions { - unitTests.all { - enabled = false - } - } compileSdk 35 defaultConfig { @@ -25,12 +20,6 @@ android { buildConfigField "String", "ZOTERO_OAUTH_CLIENT_SECRET", "\"${System.getenv('ZOTERO_OAUTH_CLIENT_SECRET') ?: 'YOUR_CLIENT_SECRET_HERE'}\"" } - testOptions { - unitTests.all { - enabled = false - } - } - buildTypes { release { minifyEnabled false @@ -48,8 +37,8 @@ android { } lint { - abortOnError false - checkReleaseBuilds false + abortOnError true + checkReleaseBuilds true } } diff --git a/app/src/main/java/oyvindbs/zotshelf/CollectionFragment.java b/app/src/main/java/oyvindbs/zotshelf/CollectionFragment.java index f186293..2b5ce70 100644 --- a/app/src/main/java/oyvindbs/zotshelf/CollectionFragment.java +++ b/app/src/main/java/oyvindbs/zotshelf/CollectionFragment.java @@ -146,9 +146,10 @@ private void loadCovers() { new EpubCoverRepository.CoverRepositoryCallback() { @Override public void onCoversLoaded(List cachedCovers) { - if (getActivity() == null) return; + Activity activity = getActivity(); + if (activity == null) return; - getActivity().runOnUiThread(() -> { + activity.runOnUiThread(() -> { if (!cachedCovers.isEmpty()) { updateUI(cachedCovers); @@ -176,9 +177,10 @@ public void onCoversLoaded(List cachedCovers) { @Override public void onError(String message) { - if (getActivity() == null) return; + Activity activity = getActivity(); + if (activity == null) return; - getActivity().runOnUiThread(() -> { + activity.runOnUiThread(() -> { Log.e("CollectionFragment", "Error loading cached covers: " + message); if (NetworkUtils.isNetworkAvailable(requireContext())) { loadCoversFromApi(); @@ -245,9 +247,10 @@ public void onSuccess(List zoteroItems) { public void onError(String errorMessage) { Log.e("CollectionFragment", "=== API ERROR ==="); Log.e("CollectionFragment", "Error: " + errorMessage); - if (getActivity() == null) return; + Activity activity = getActivity(); + if (activity == null) return; - getActivity().runOnUiThread(() -> { + activity.runOnUiThread(() -> { progressBar.setVisibility(View.GONE); swipeRefreshLayout.setRefreshing(false); @@ -288,8 +291,9 @@ public void onSuccess(List zoteroItems) { zoteroItems.size() + " items from API"); processZoteroItemsForCache(zoteroItems); - if (getActivity() == null) return; - getActivity().runOnUiThread(() -> { + Activity activity = getActivity(); + if (activity == null) return; + activity.runOnUiThread(() -> { swipeRefreshLayout.setRefreshing(false); Toast.makeText(requireContext(), "Library updated from Zotero", Toast.LENGTH_SHORT).show(); @@ -299,9 +303,10 @@ public void onSuccess(List zoteroItems) { @Override public void onError(String errorMessage) { Log.e("CollectionFragment", "Background update error: " + errorMessage); - if (getActivity() == null) return; + Activity activity = getActivity(); + if (activity == null) return; - getActivity().runOnUiThread(() -> { + activity.runOnUiThread(() -> { swipeRefreshLayout.setRefreshing(false); }); } @@ -343,9 +348,10 @@ private void loadCachedCovers() { new EpubCoverRepository.CoverRepositoryCallback() { @Override public void onCoversLoaded(List covers) { - if (getActivity() == null) return; + Activity activity = getActivity(); + if (activity == null) return; - getActivity().runOnUiThread(() -> { + activity.runOnUiThread(() -> { if (covers.isEmpty()) { showEmptyState("No cached covers found"); } else { @@ -361,9 +367,10 @@ public void onCoversLoaded(List covers) { @Override public void onError(String message) { - if (getActivity() == null) return; + Activity activity = getActivity(); + if (activity == null) return; - getActivity().runOnUiThread(() -> { + activity.runOnUiThread(() -> { showEmptyState("Error loading cached covers: " + message); swipeRefreshLayout.setRefreshing(false); }); @@ -373,9 +380,10 @@ public void onError(String message) { private void processZoteroItems(List zoteroItems) { if (zoteroItems.isEmpty()) { - if (getActivity() == null) return; + Activity activity = getActivity(); + if (activity == null) return; - getActivity().runOnUiThread(() -> { + activity.runOnUiThread(() -> { progressBar.setVisibility(View.GONE); swipeRefreshLayout.setRefreshing(false); @@ -497,9 +505,10 @@ public void onError(ZoteroItem item, String errorMessage) { } private void updateUI(final List newItems) { - if (getActivity() == null) return; + Activity activity = getActivity(); + if (activity == null) return; - getActivity().runOnUiThread(() -> { + activity.runOnUiThread(() -> { coverItems.clear(); coverItems.addAll(newItems); @@ -577,9 +586,10 @@ public void updateDisplayMode() { } public void applySorting() { - if (getActivity() == null) return; + Activity activity = getActivity(); + if (activity == null) return; - getActivity().runOnUiThread(() -> { + activity.runOnUiThread(() -> { if (!coverItems.isEmpty()) { // Apply current sort mode int sortMode = userPreferences.getSortMode(); @@ -601,9 +611,10 @@ public void applySorting() { * Show an error dialog */ private void showErrorDialog(String title, String message) { - if (getActivity() == null) return; + Activity activity = getActivity(); + if (activity == null) return; - AlertDialog.Builder builder = new AlertDialog.Builder(requireContext()); + AlertDialog.Builder builder = new AlertDialog.Builder(activity); builder.setTitle(title); builder.setMessage(message); builder.setPositiveButton("OK", null); diff --git a/app/src/main/java/oyvindbs/zotshelf/ZoteroApiClient.java b/app/src/main/java/oyvindbs/zotshelf/ZoteroApiClient.java index 21f267c..259dcc0 100644 --- a/app/src/main/java/oyvindbs/zotshelf/ZoteroApiClient.java +++ b/app/src/main/java/oyvindbs/zotshelf/ZoteroApiClient.java @@ -209,8 +209,7 @@ public void getEpubItems(String userId, String apiKey, ZoteroCallback> callback) { executor.execute(() -> { - Log.d(TAG, "Getting collections - UserId: '" + userId + "', API Key: '" + - (apiKey != null ? apiKey.substring(0, 5) + "..." : "null") + "'"); + Log.d(TAG, "Getting collections - UserId: '" + userId + "', API Key: ***MASKED***"); if (userId == null || userId.isEmpty()) { Log.e(TAG, "User ID is empty or null!"); @@ -386,7 +385,13 @@ public void downloadEpub(ZoteroItem item, FileCallback callback) { callback.onFileDownloaded(item, epubFile.getAbsolutePath()); return; } - + + if (item.getLinks() == null || item.getLinks().getEnclosure() == null || + item.getLinks().getEnclosure().getHref() == null) { + callback.onError(item, "Download URL not available"); + return; + } + String downloadUrl = item.getLinks().getEnclosure().getHref(); String apiKey = new UserPreferences(context).getZoteroApiKey(); @@ -413,26 +418,18 @@ public void downloadEpub(ZoteroItem item, FileCallback callback) { } private boolean writeResponseBodyToDisk(ResponseBody body, File outputFile) { - try { - InputStream inputStream = body.byteStream(); - OutputStream outputStream = new FileOutputStream(outputFile); - + try (InputStream inputStream = body.byteStream(); + OutputStream outputStream = new FileOutputStream(outputFile)) { + byte[] fileReader = new byte[4096]; - - while (true) { - int read = inputStream.read(fileReader); - - if (read == -1) { - break; - } - + int read; + + while ((read = inputStream.read(fileReader)) != -1) { outputStream.write(fileReader, 0, read); } - + outputStream.flush(); - outputStream.close(); - inputStream.close(); - + return true; } catch (IOException e) { Log.e(TAG, "File write error", e); @@ -633,11 +630,12 @@ public void downloadEbook(ZoteroItem item, FileCallback callback) { return; } - if (item.getLinks() == null || item.getLinks().getEnclosure() == null) { + if (item.getLinks() == null || item.getLinks().getEnclosure() == null || + item.getLinks().getEnclosure().getHref() == null) { callback.onError(item, "No download link available"); return; } - + String downloadUrl = item.getLinks().getEnclosure().getHref(); String apiKey = new UserPreferences(context).getZoteroApiKey(); diff --git a/app/src/main/res/xml/network_security_config.xml b/app/src/main/res/xml/network_security_config.xml index f12d5f2..8890ffd 100644 --- a/app/src/main/res/xml/network_security_config.xml +++ b/app/src/main/res/xml/network_security_config.xml @@ -1,7 +1,7 @@ - - + + From a80b42d82116956caad7c30dbf861ed0b3d10e7a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Nov 2025 21:32:48 +0000 Subject: [PATCH 2/3] Add missing Activity import to CollectionFragment --- app/src/main/java/oyvindbs/zotshelf/CollectionFragment.java | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/main/java/oyvindbs/zotshelf/CollectionFragment.java b/app/src/main/java/oyvindbs/zotshelf/CollectionFragment.java index 2b5ce70..2e1e4a5 100644 --- a/app/src/main/java/oyvindbs/zotshelf/CollectionFragment.java +++ b/app/src/main/java/oyvindbs/zotshelf/CollectionFragment.java @@ -1,5 +1,6 @@ package oyvindbs.zotshelf; +import android.app.Activity; import android.content.Intent; import android.net.Uri; import android.os.Bundle; From 4388e9e3fd4455dfb5d3c2215e51c6de7e3a6ea7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 2 Dec 2025 20:27:52 +0000 Subject: [PATCH 3/3] Add copy link button to book covers with configurable link type This commit adds a copy button to each book cover that copies either a web library link or internal Zotero link to the clipboard, based on user preference. Features: - Added small copy button (icon) in the top-right corner of each book cover - New setting in Settings to choose between web library links and internal Zotero links - Web link format: https://www.zotero.org/{username}/items/{itemId} - Internal link format: zotero://select/library/items/{itemId} - Toast notification confirms which link type was copied Implementation: - Added LINK_TYPE_WEB and LINK_TYPE_INTERNAL constants to UserPreferences - Added getLinkType() and setLinkType() methods to UserPreferences - Added link type radio group to Settings layout - Created ic_copy.xml vector drawable (white copy icon) - Modified grid_item_cover.xml to add ImageButton overlaying the cover image - Updated CoverGridAdapter to handle copy button clicks and clipboard operations - Updated SettingsActivity to load/save link type preference User Experience: - Small, unobtrusive button in corner of each cover - Copies link instantly with visual feedback via toast - User can configure link type once in settings --- .../oyvindbs/zotshelf/CoverGridAdapter.java | 40 +++++++++++++++++++ .../oyvindbs/zotshelf/SettingsActivity.java | 24 +++++++++++ .../oyvindbs/zotshelf/UserPreferences.java | 17 +++++++- app/src/main/res/drawable/ic_copy.xml | 9 +++++ app/src/main/res/layout/activity_settings.xml | 36 +++++++++++++++++ app/src/main/res/layout/grid_item_cover.xml | 31 +++++++++++--- 6 files changed, 149 insertions(+), 8 deletions(-) create mode 100644 app/src/main/res/drawable/ic_copy.xml diff --git a/app/src/main/java/oyvindbs/zotshelf/CoverGridAdapter.java b/app/src/main/java/oyvindbs/zotshelf/CoverGridAdapter.java index db58eb5..bcc616d 100644 --- a/app/src/main/java/oyvindbs/zotshelf/CoverGridAdapter.java +++ b/app/src/main/java/oyvindbs/zotshelf/CoverGridAdapter.java @@ -1,11 +1,15 @@ package oyvindbs.zotshelf; +import android.content.ClipData; +import android.content.ClipboardManager; import android.content.Context; import android.view.LayoutInflater; import android.view.View; import android.view.ViewGroup; +import android.widget.ImageButton; import android.widget.ImageView; import android.widget.TextView; +import android.widget.Toast; import androidx.annotation.NonNull; import androidx.recyclerview.widget.RecyclerView; @@ -85,6 +89,11 @@ public void onBindViewHolder(@NonNull CoverViewHolder holder, int position) { listener.onCoverClick(item); } }); + + // Set copy button click listener + holder.copyButton.setOnClickListener(v -> { + copyLinkToClipboard(item); + }); } @Override @@ -92,14 +101,45 @@ public int getItemCount() { return coverItems.size(); } + private void copyLinkToClipboard(EpubCoverItem item) { + UserPreferences userPreferences = new UserPreferences(context); + int linkType = userPreferences.getLinkType(); + String link; + String linkLabel; + + if (linkType == UserPreferences.LINK_TYPE_INTERNAL) { + // Internal Zotero link: zotero://select/library/items/{itemId} + link = "zotero://select/library/items/" + item.getId(); + linkLabel = "Internal link"; + } else { + // Web library link: https://www.zotero.org/{username}/items/{itemId} + String username = item.getZoteroUsername(); + if (username == null || username.isEmpty()) { + username = userPreferences.getZoteroUsername(); + } + link = "https://www.zotero.org/" + username + "/items/" + item.getId(); + linkLabel = "Web link"; + } + + // Copy to clipboard + ClipboardManager clipboard = (ClipboardManager) context.getSystemService(Context.CLIPBOARD_SERVICE); + ClipData clip = ClipData.newPlainText(linkLabel, link); + clipboard.setPrimaryClip(clip); + + // Show toast notification + Toast.makeText(context, linkLabel + " copied to clipboard", Toast.LENGTH_SHORT).show(); + } + static class CoverViewHolder extends RecyclerView.ViewHolder { ImageView coverImage; TextView titleText; + ImageButton copyButton; public CoverViewHolder(@NonNull View itemView) { super(itemView); coverImage = itemView.findViewById(R.id.imageCover); titleText = itemView.findViewById(R.id.textTitle); + copyButton = itemView.findViewById(R.id.buttonCopyLink); } } } \ No newline at end of file diff --git a/app/src/main/java/oyvindbs/zotshelf/SettingsActivity.java b/app/src/main/java/oyvindbs/zotshelf/SettingsActivity.java index c24a9a0..605c3f0 100644 --- a/app/src/main/java/oyvindbs/zotshelf/SettingsActivity.java +++ b/app/src/main/java/oyvindbs/zotshelf/SettingsActivity.java @@ -29,6 +29,9 @@ public class SettingsActivity extends AppCompatActivity { private RadioButton radioTitleOnly; private RadioButton radioAuthorOnly; private RadioButton radioAuthorTitle; + private RadioGroup radioGroupLinkType; + private RadioButton radioLinkWeb; + private RadioButton radioLinkInternal; private Button buttonSave; private Button buttonOAuthLogin; private UserPreferences userPreferences; @@ -58,6 +61,9 @@ protected void onCreate(Bundle savedInstanceState) { radioTitleOnly = findViewById(R.id.radioTitleOnly); radioAuthorOnly = findViewById(R.id.radioAuthorOnly); radioAuthorTitle = findViewById(R.id.radioAuthorTitle); + radioGroupLinkType = findViewById(R.id.radioGroupLinkType); + radioLinkWeb = findViewById(R.id.radioLinkWeb); + radioLinkInternal = findViewById(R.id.radioLinkInternal); buttonSave = findViewById(R.id.buttonSave); buttonOAuthLogin = findViewById(R.id.buttonOAuthLogin); @@ -121,6 +127,14 @@ private void loadPreferences() { radioTitleOnly.setChecked(true); break; } + + // Set the link type radio button + int linkType = userPreferences.getLinkType(); + if (linkType == UserPreferences.LINK_TYPE_INTERNAL) { + radioLinkInternal.setChecked(true); + } else { + radioLinkWeb.setChecked(true); + } } private void savePreferences() { @@ -166,6 +180,16 @@ private void savePreferences() { } userPreferences.setDisplayMode(displayMode); + // Save link type + int linkType; + int selectedLinkTypeId = radioGroupLinkType.getCheckedRadioButtonId(); + if (selectedLinkTypeId == R.id.radioLinkInternal) { + linkType = UserPreferences.LINK_TYPE_INTERNAL; + } else { + linkType = UserPreferences.LINK_TYPE_WEB; + } + userPreferences.setLinkType(linkType); + Toast.makeText(this, R.string.settings_saved, Toast.LENGTH_SHORT).show(); finish(); } diff --git a/app/src/main/java/oyvindbs/zotshelf/UserPreferences.java b/app/src/main/java/oyvindbs/zotshelf/UserPreferences.java index 390d00f..d0e14b8 100644 --- a/app/src/main/java/oyvindbs/zotshelf/UserPreferences.java +++ b/app/src/main/java/oyvindbs/zotshelf/UserPreferences.java @@ -16,7 +16,8 @@ public class UserPreferences { private static final String KEY_SHOW_PDFS = "show_pdfs"; private static final String KEY_BOOKS_ONLY = "books_only"; private static final String KEY_SORT_MODE = "sort_mode"; - + private static final String KEY_LINK_TYPE = "link_type"; + // Display mode constants public static final int DISPLAY_TITLE_ONLY = 0; public static final int DISPLAY_AUTHOR_ONLY = 1; @@ -25,7 +26,11 @@ public class UserPreferences { // Sort mode constants public static final int SORT_BY_TITLE = 0; public static final int SORT_BY_AUTHOR = 1; - + + // Link type constants + public static final int LINK_TYPE_WEB = 0; + public static final int LINK_TYPE_INTERNAL = 1; + private final SharedPreferences preferences; public UserPreferences(Context context) { @@ -130,6 +135,14 @@ public boolean hasAnyFileTypeEnabled() { return getShowEpubs() || getShowPdfs(); } + public int getLinkType() { + return preferences.getInt(KEY_LINK_TYPE, LINK_TYPE_WEB); // Default to web link + } + + public void setLinkType(int linkType) { + preferences.edit().putInt(KEY_LINK_TYPE, linkType).apply(); + } + public void clearAll() { preferences.edit().clear().apply(); } diff --git a/app/src/main/res/drawable/ic_copy.xml b/app/src/main/res/drawable/ic_copy.xml new file mode 100644 index 0000000..3918c76 --- /dev/null +++ b/app/src/main/res/drawable/ic_copy.xml @@ -0,0 +1,9 @@ + + + diff --git a/app/src/main/res/layout/activity_settings.xml b/app/src/main/res/layout/activity_settings.xml index 6365f5a..3ba67cb 100644 --- a/app/src/main/res/layout/activity_settings.xml +++ b/app/src/main/res/layout/activity_settings.xml @@ -261,6 +261,42 @@ + + + + + + + + + + + +