[expo-updates] add ability for android clients to handle header signatures #11897
Conversation
|
Native Component List for this branch is ready |
| preManifest.put("isVerified", false); | ||
| Manifest manifest = ManifestFactory.getManifest(preManifest, configuration); |
There was a problem hiding this comment.
Previously we only set isVerified to false on unsigned manifests from XDL. isVerified was never set on unsigned manifests from non-XDL sources. Grep'ing the expo repo it seems this is handled by interpreting null to be false using optBoolean.
Following 'explicit is better than implicit' as well as this code comment:
// We should treat these manifests as unsigned rather than signed with an invalid signature.
This PR centralizes the treatment of unsigned manifests from XDL and non-XDL sources to these lines.
| @@ -204,7 +213,7 @@ public void onResponse(Call call, Response response) throws IOException { | |||
| }); | |||
| } | |||
|
|
|||
| private static JSONObject extractManifest(String manifestString, UpdatesConfiguration configuration) throws IOException { | |||
| private static JSONObject extractUpdateResponseJson(String manifestString, UpdatesConfiguration configuration) throws IOException { | |||
There was a problem hiding this comment.
This function seems like it is dead code. It is unable to handle the array case in the case that the manifest is signed.
Is there a use case for arrays of sdkVersions only in the case of unsigned manifests?
Otherwise we could just replace this with a call to JSONObject.
There was a problem hiding this comment.
Is there a use case for arrays of
sdkVersionsonly in the case of unsigned manifests?
Yes, exactly -- the "array" case is used for multi-manifests generated by expo export, which are never signed. While we could add the ability to handle signatures in the body, I think it's better to just focus on supporting the modern method of header signatures moving forward.
packages/expo-updates/android/src/main/java/expo/modules/updates/loader/FileDownloader.java
Outdated
Show resolved
Hide resolved
| @@ -204,7 +213,7 @@ public void onResponse(Call call, Response response) throws IOException { | |||
| }); | |||
| } | |||
|
|
|||
| private static JSONObject extractManifest(String manifestString, UpdatesConfiguration configuration) throws IOException { | |||
| private static JSONObject extractUpdateResponseJson(String manifestString, UpdatesConfiguration configuration) throws IOException { | |||
There was a problem hiding this comment.
Is there a use case for arrays of
sdkVersionsonly in the case of unsigned manifests?
Yes, exactly -- the "array" case is used for multi-manifests generated by expo export, which are never signed. While we could add the ability to handle signatures in the body, I think it's better to just focus on supporting the modern method of header signatures moving forward.
…es/loader/FileDownloader.java Co-authored-by: Eric Samelson <esamelson@users.noreply.github.com>
Why
EAS updates will send the signature back in the headers.
Working on PR for the iOS version separately.
How
extractManifesttoextractUpdateResponseJsonTest Plan
Tested signed/unsigned on new/legacy format manifests.