BaseMax / GooglePlayWebServiceAPI

Tiny script to crawl information of a specific application in the Google play/store base on PHP.
MIT License
37 stars 9 forks source link

parsePerms(): work around broken ProtoBuf for some (very few) apps #25

Closed IzzySoft closed 1 year ago

IzzySoft commented 1 year ago

Looks like there are rare conditions with "misc" perms having no group name. This commit should mitigate that hopefully. Maybe wait a day or two before merging: I've just put it to production here so it hopefully gets confirmed (at least to have no side-effects :wink:)

BaseMax commented 1 year ago

Thanks Izzy. Please let me know when you think it is ready to go.

IzzySoft commented 1 year ago

Sure, will do. I had that funny warning now two days in a row (which caused me to make this adjustment), but don't remember having seen it ever before – so I do not even have any idea what time I can say "that's it". Basically I see no reason to hold back: check the diff, the only difference is that it now first confirms the array is really there and has data before accessing it – and otherwise leaving that field empty. Which is the same as before, just without the notice thrown: if the array was not there or not set, the field was left empty implicitly :man_shrugging:

With hundreds of apps per scan here, I cannot tell which one triggered it and thus cannot test that case. I've tested it with some samples, though: app with no perms at all, app with lots of perms, apps with perms but none in "misc" (which should have addressed this one), so it should be OK.

Just hit the button if I don't report back within a week I'd say :see_no_evil: My "hesitation" is just "overly cautiousness". I have it live already here, but that's my data – merging it here affects other people's data :wink:

IzzySoft commented 1 year ago

Parsing ProtoBuf without the defs is try-and-err :see_no_evil: Good we waited. Let's give it a few more days for the update to confirm itself. Should fit now with this additional commit, but who knows (maybe the else part should be omitted entirely, not setting misc at all if there is no misc? The other sections aren't set either if there's nothing in)…

IzzySoft commented 1 year ago

Still something wrong. Investigating. Maybe Google changed something in the ProtoBuf structures a few days ago, as this now gets triggered daily here while it was not triggered ever before…

IzzySoft commented 1 year ago

OK, after I let my updater run in circles half a day with some debug info placed, I finally found a candidate to check against: com.achunt.weboslauncher is triggering the error. According to PlayStore (here Appbrain, where the permission section is easier to get at), the app has two permissions: android.permission.QUERY_ALL_PACKAGES and "choose widgets". But the protobuf seems incomplete:

)]}'

137
[["wrb.fr","xdSrCf","[null,null,[[null,\"choose widgets\"]]]",null,null,null,"1"],["di",72],["af.httprm",72,"-1523241701239662311",15]]
25
[["e",4,null,null,173]]

No trace of the first permission. Interpreting that as JSON (which is the work-around I use here):

[
  [
    "wrb.fr", "xdSrCf",
    "[null,null,[[null,\"choose widgets\"]]]",
    null, null, null, "1"
  ],[
    "di", 72
  ],[
    "af.httprm", 72, "-1523241701239662311", 15
  ]
]

$arr[1][0] is not, as expected, an array – but a literal (di), so we cannot obtain the "group name" (which should have been "Your personal information"). Easiest approach would be making the else branch

    else $perms['misc'] = ['group_name'=>'unknown', 'perms'=>$arr[2]];

to not lose the "group section" entirely. The bigger issue I see is we miss the other permission, so our results are not reliable. As that seems to happen in not much more than 1 out of 1.000 cases, can we live with that? OTOH, what other choice do we have?

Huh? :rofl: Funny to see, Playstore itself has the very same issue – so it's not us, it's really them having messed up their data:

image

which means Appbrain seems to obtain the APK and run aapt dump badging on it itself, else it could not have the full list of permissions. As the app is in my own repo as well, I just did the same:

package: name='com.achunt.weboslauncher' versionCode='22' versionName='1.1' platformBuildVersionName='12' platformBuildVersionCode='32' compileSdkVersion='32' compileSdkVersionCodename='12'
sdkVersion:'26'
targetSdkVersion:'32'
uses-permission: name='android.permission.QUERY_ALL_PACKAGES'
uses-permission: name='android.permission.BIND_APPWIDGET'
application-label:'W Launcher'

So I'd say I go with the work-around proposed, and we hope Google will fix it on their end. Do you agree? While waiting for your confirmation, I'll now establish that work-around on my end for some "control runs" (some hundred apps 4 times a day should give us some confidence then).

BaseMax commented 1 year ago

@BaseMax BaseMax marked this pull request as ready for review now

IzzySoft commented 1 year ago

@BaseMax running smooth here since, I'd say go for it.

BaseMax commented 1 year ago

Thanks. @IzzySoft

BaseMax commented 1 year ago

And what do you think about elseif token instead of else if token?

IzzySoft commented 1 year ago

Uh? It uses elseif, doesn't it?