search

electrode-io / electrode-ota-server

Electrode Over The Air Server for hot deployment of React Native and Cordova mobile apps
Other
204 stars 40 forks source link

Fix acquisition when package hash is missing #70

Closed belemaire closed 6 years ago

belemaire commented 6 years ago

While doing some testing on Android, using an Electrode OTA server instance running locally on my machine, from current master, I noticed an issue in updateCheck acquisition function, leading to an HTTP 500 internal server error when trying to acquire a new package.

1) Push new pkg1 to server 2) Launch Android app. updateCheck OK. Android app properly retrieves pkg1 (Going from binary bundle => pkg1). 3) Push new pkg2 to server. 4) Launch Android app. updateCheck OK. Android app properly retrieves pkg2 (Going from pkg1 => pkg2) 5) Uninstall Android app and reinstall it from scratch, then launch it. While it should go from binary bundle => pkg2, updateCheck is failing with internal server error.

The error happens because of the insertPackageContent SQL request that is failing due to same package hash (primary key) already existing. This is because for some reason, with the current code, when the packageHash query parameter has no value when calling updateCheck, the code reaches this point apparently creating a diff of latest package v.s installed package. The problem here is that in the case of a missing packageHash param, the latest package and installed package are the same for some reason, leading to creating a diff between two identical packages. This happens because in step 2) above, the packageHash is missing leading to the creation of a diff file with no diff, and hashed. Then in step 5 the same is done, leading to the same diff hash.

Anyway, I can see that these modifications were done in recent PR https://github.com/electrode-io/electrode-ota-server/pull/68, to fix issue https://github.com/electrode-io/electrode-ota-server/issues/66. This current PR still works for https://github.com/electrode-io/electrode-ota-server/issues/66 as in the logs provided we can see that a hash exists.

For Android, when a new native application version is installed on the phone, the first time updateCheck will be called, will always be with an empty/missing packageHash query parameter, due to some technical restriction. Therefore, an empty packageHash means "I am running this native application for the first time, so I have not yet downloaded any update". In that case, the latest available package for this version should be returned, which is what is done by short-circuiting the if block.

Tested with same scenario, and working fine with this fix.

You should not run 4.4.7 on Production as it will lead to this issue for Android updates.

awhelms commented 6 years ago

I don't have a problem with the code change, but I did notice you referenced an error happening when running insertPackageContent. I'd caution anyone against using the database in any production capacity to hold the package content. It is preferable to use a file service and override the fileservice upload and download modules. This makes a difference on downloads as MariaDB will put blobs into a Buffer instead of allowing streaming.

datvong-wm commented 6 years ago

Great investigation! I'll update the server

belemaire commented 6 years ago

@awhelms Thanks for your remark ! I'm just in the early stages of getting familiar with the code base and the design. My goal was to get a server up and running quickly on my local workstation without too much hassle. I was able to do it -with a little bit of hassle still, not so much doc to setup a server, so just looking at a reference config, the codebase and trying things out-, but in term of configuration I took some shortcuts, so didn't configure a file service for upload/download, that's why I ended up with DB file storage by default I guess, which is OK for my needs of just having a local server for playing around. Definitely not a production setup ;)