Skip to content
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.

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

Fix AutoUpload notification #3173

Merged
merged 1 commit into from
Oct 24, 2018
Merged

Fix AutoUpload notification #3173

merged 1 commit into from
Oct 24, 2018

Conversation

tobiasKaminsky
Copy link
Member

fix #3163

  • fix not working detection of already defined synced folders
  • delete not used (and wrong) function

Problem was that passing everything into selection did not worked.
Also this is error prone to sql injection (yes, very theoratical with folder names), but changing it to use correct parameter solves both.

Steps to test:

  • have images in camera folder
  • make sure that download folder is not in media scanner (delete images from download in gallery app)
  • clean installation of app
  • set up camera folder
  • take image -> no notification
  • delete all files in camera folder
  • take image -> no notification
  • download image e.g. with Firefox --> see notification

Signed-off-by: tobiasKaminsky tobias@kaminsky.me

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Oct 23, 2018

👍

Approved with PullApprove

@AndyScherzinger
Copy link
Member

Problem was that passing everything into selection did not worked.

Can you give me some hints here since looking at the code it kind of looks the same to me as before except for the query change using parameters. I am pretty sure I am just overlooking something looking at the diff...

@tobiasKaminsky
Copy link
Member Author

I trimmed it a bit for better readability.

Old:
selection: SYNCED_FOLDER_LOCAL_PATH == localPath AND SYNCED_FOLDER_ACCOUNT == account.name
arguments: /

New:
selection: SYNCED_FOLDER_LOCAL_PATH =? AND SYNCED_FOLDER_ACCOUNT =?
arguments: new String[]{localPath, account.name}

With new system we let Android construct the query by inserting the arguments to the correct position. This prevents from SQL injection and also does correct escaping.

@AndyScherzinger
Copy link
Member

Thanks, now I can see it. 🙏

- delete not used (and wrong) function

Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>
@nextcloud-android-bot
Copy link
Collaborator

Lint

TypeMasterPR
Warnings9696
Errors

FindBugs (new)

Warning TypeNumber
Bad practice Warnings35
Correctness Warnings167
Internationalization Warnings14
Malicious code vulnerability Warnings10
Multithreaded correctness Warnings9
Performance Warnings123
Security Warnings203
Dodgy code Warnings138
Total699

FindBugs (master)

Warning TypeNumber
Bad practice Warnings35
Correctness Warnings167
Internationalization Warnings14
Malicious code vulnerability Warnings10
Multithreaded correctness Warnings9
Performance Warnings123
Security Warnings203
Dodgy code Warnings139
Total700

@codecov
Copy link

codecov bot commented Oct 24, 2018

Codecov Report

Merging #3173 into master will increase coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##             master   #3173      +/-   ##
===========================================
+ Coverage      6.36%   6.37%   +<.01%     
  Complexity        1       1              
===========================================
  Files           307     307              
  Lines         30118   30108      -10     
  Branches       4320    4317       -3     
===========================================
+ Hits           1918    1920       +2     
+ Misses        27913   27902      -11     
+ Partials        287     286       -1
Impacted Files Coverage Δ Complexity Δ
...ncloud/android/datamodel/SyncedFolderProvider.java 9.73% <0%> (+0.86%) 0 <0> (ø) ⬇️
...wncloud/android/jobs/MediaFoldersDetectionJob.java 8.33% <0%> (-0.12%) 0 <0> (ø)
.../third_parties/daveKoeller/AlphanumComparator.java 82.14% <0%> (-1.2%) 0% <0%> (ø)
...in/java/com/owncloud/android/datamodel/OCFile.java 63.42% <0%> (+1.16%) 0% <0%> (ø) ⬇️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto Upload Notification recreation
3 participants