pkumza / LibRadar

LibRadar - A detecting tool for 3rd-party libraries in Android apps.
Apache License 2.0
256 stars 51 forks source link

Retagging too broad and slightly off definitions #36

Closed IzzySoft closed 7 years ago

IzzySoft commented 7 years ago

For reference, see #33 – I just left out Mozilla:

Let it sit for a little (after merging – for now I'm through). Then it might be worth another final (if minor) release of the V1 branch. After all, an almost complete overhaul of the database taggings, plus a scan speedup should justify that. We might just want to wait a few days not that anything else pops up right after that release :smile_cat:

IzzySoft commented 7 years ago

OK, done! I've just rescanned my entire APK base in just about 2 hours (before adding that parameter to apktool that would have been at least 6 hours – so my feeling was right, and I rather understated the factor which seems like × 3!) After about 600 files en block I'd say it's stable, got some more gaps filled – and (as indicated above) should be made another V1 release, if you agree :innocent:

Thanks for your support! Next PR from my end will not be that soon again (i.e. no more PRs from me this week :laughing:).

IzzySoft commented 7 years ago

A note on Codehaus: da;Codehaus is definitely wrong. Codehaus (now shut down) was something like Github or Sourceforge. Tagging libraries "Codehaus" because they start with "com/codehaus" is like tagging everything Github that starts with "com/github" (check it out :)

There are still a lot of those da;Codehaus entries present, so this still needs a hand.

pkumza commented 7 years ago

Hello, In the commit 8c286f2. com/melnykov/fab,FloatingActionButton should be a GUI Component instead of utility right?

IzzySoft commented 7 years ago

FloatingActionButton should be a GUI Component instead of utility right?

It's in between – but yes, probably rather UI than UT. Shall I retag and push?

Should we tagged the libs before remove the tag for Polidea?

I had taken that approach if I knew what to tag it to. OTOH, it's just 3 entries which I've never hit (I just encountered a few completely different Polidea projects with at least one of them starting with pl/polidea) – so I felt it wouldn't have that big an impact. Was different for Mozilla which I didn't untag (but retagged some sub-projects), as it had quite a lot of entries.

As with the FAB, if you want me to revert the untagging of Polidea¹, just let me know.

¹ Just found it: "ut;android-zoom-view;https://github.com/Polidea/android-zoom-view". Or shall it be UI? From the (no longer maintained) project: "Free and efficient widget for creating zoomable applications." Widget -> UI. To create applications -> UT. Your choice :stuck_out_tongue_winking_eye:

PS: This morning I finished the "backlog" (results pending from that "mass processing" of ~600 .apk files) which caused the latest commit batch – so apart from above 2 items and any open questions you might have, the PR should be ready to be processed. Once done, I'd have one more PR in mind on V1 (updating apktool.jar as I already indicated) – after which I'd suggest to let it settle for a week and, if no further adjustments come up, make another "last V1 release"?

pkumza commented 7 years ago

If a lib is something between ut & GUI, I prefer tag it as GUI. As GUI libraries are somehow, a kind of utilities in broad sense.

I think it might be better to revert the untagging of Polidea before we found library instances.

Yeah, it will be settled this week! Newest Android.jar is needed and I will give a stable release for V1 later if it passes some tests.

pkumza commented 7 years ago

com/melnykov/fab is another GUI lib in my opinion, would you like to retag it and make another merge request?

IzzySoft commented 7 years ago

I think it might be better to revert the untagging of Polidea before we found library instances.

Seems you've missed that I've identified the 3 today. I will retag Polidea accordingly, and push that tonight – so consider "Polidea fixed".

Newest Android.jar is needed and I will give a stable release for V1 later if it passes some tests.

Android.jar? You mean the apktool.jar? If it's that, I'm using the January version of it for a couple of months now – and have less issues than with the one shipping in V1 (in fact, I only had a single issue with just one .apk, and that might well have been the APK's fault). So I could push the version I've thoroughly tested, if you want me to.

com/melnykov/fab is another GUI lib in my opinion, would you like to retag it and make another merge request?

Sure, no problem. As soon as you've merged this one, I'll do a fetch and merge on my end, then fix the things we've just discussed (Polidea, FAB, check your "squareup libs" – btw: I had those tagged? I for sure remember seismic and pollexor), and make it another PR. That one merged and, if you want me, I'd do a PR for apktool.jar.

IzzySoft commented 7 years ago

Shall I still push to this PR (as it's still open) – or do you want to merge it first (as you've already reviewed)? I just want to avoid causing trouble if pushing to a half-merged PR :innocent:

Ah, OK, still works. Going on then with the next one.

IzzySoft commented 7 years ago

OK, that should be it then for the retagging. I've checked those com/squareup/* from above, and I already had tagged them. Just tagged the only missing one I was able to identify (com/squareup/javawriter). The only one left now is com/squareup/a, which you had tagged "pa". Hard to say for a "speaking name" like that what package it was in.

Sure there are plenty items left untagged – but I cannot do them all. I've always tagged those I encountered in my packages, and will continue doing so. But for this PR with now 29 commits, this should be all, sorry :stuck_out_tongue_winking_eye:

pkumza commented 7 years ago

I've already viewed these commits.

However, one merge would be clear. So, keep pushing to this PR before main broad libs are retagged if you don't mind?🙂

IzzySoft commented 7 years ago

So, keep pushing to this PR before main broad libs are retagged if you don't mind?

As long as it's open, I will do so. But as I'm through the batch, I doubt there will be much to push within the next days. On the other hand, as soon as this PR is merged I'd like to make another PR for the apktool.jar, as discussed. With that done, V1 would be in a good and consistent state again – and I could start another "tagging batch" / PR anytime. Currently, with "my job done", I'd prefer concentrating on other waiting tasks :wink:

pkumza commented 7 years ago

Btw I just made a Lite version for LibRadar in this master branch which do not have Redis dependency. You could have a try if you want.

pkumza commented 7 years ago

I've merged these commits and This PR is closed... Please open another PR soon ~ 😃

IzzySoft commented 7 years ago

Btw I just made a Lite version for LibRadar in this master branch which do not have Redis dependency. You could have a try if you want.

That sounds great! Will check that as soon as time permits. I was already looking towards Docker and whether to build a fitting container – but a Lite version certainly makes things easier :smile_cat:

BTW: If I switch to V2/Lite, how can I contribute definitions (as with this PR)? Or even scan .apk files I know contain libraries not yet covered by LibRadar? I vaguely remember you've planned something for that with V2 – a pointer/link would be appreciated!

Please open another PR soon

Will do, though I'm not sure whether I manage that today (will be a long day, and I'm unlikely to be home before 10pm). But the next PR is a small one, just that one .jar file, so it shouldn't take more than a few minutes, and done latest tomorrow :innocent:

pkumza commented 7 years ago

If I switch to V2/Lite, how can I contribute definitions (as with this PR)?

Yes! You can just change tag_rules.csv to contribute or update definitions.

Or even scan .apk files I know contain libraries not yet covered by LibRadar?

I've support this functionality in V2 but it's in V2/Ordinary.😎 (Search 'brand' function and you could find it) It will not be hard to migrate it into V2/Lite (Need some time).

IzzySoft commented 7 years ago

You can just change tag_rules.csv to contribute or update definitions.

That means here I could also add new definitions? I've collected quite a few, though in a different format. So I e.g. have an additional column: additional to the package name, I have the "path to match" – which is required for several libraries to keep them apart (e.g. one cannot simply use com/facebook, as there are multiple different libraries using that prefix. Examples:

com/facebook,com/facebook/appevents
com/facebook,com/facebook/applinks
com/facebook,com/facebook/devicerequests
...
com/facebook/fresco,com/facebook/drawable
com/facebook/fresco,com/facebook/drawee
com/facebook/fresco,com/facebook/imageutils
...
com/facebook/stetho,com/facebook/stetho

As you can see by those examples, I felt the need to map multiple "paths" to the same "package name". With the V1 format, this would be one pn with multiple spn. How is that dealt with in V2? I cannot find a file with such mappings. So I wonder if adding to tag_rules.csv alone would be sufficient – as from the above list, I could only add 3 items (by their package name) but not the "bindings" to match.

Speaking of "bindings": I wonder that you have Lbutterknife tagged "GUI Component". That's rather a utility: "Field and method binding for Android views". Sure it deals with views, but rather with binding methods to them than with the GUI component.