接触チェックに漏れが発生する可能性について #17

yoshitomo-g · 2021-01-20T10:54:01Z

iOS版のコードを追いかけていて気が付いたのですが、zipファイルのダウンロード完了後に処理が中断した場合、この時のzipファイルに含まれる診断キーとの接触チェックが行われずに漏れてしまうように思えます。具体的には、次の行以降です。

cocoa/Covid19Radar/Covid19Radar/Services/ExposureNotificationHandler.cs

Line 164 in 3eac4f4

 var (batchNumber, downloadedFiles) = await DownloadBatchAsync(serverRegion, cancellationToken); 

この行のメソッドでは、各zipファイルのCreatedの値を、最後にダウンロードしたzipファイルのCreatedであるlastTekTimestamp[region]と比較し、ダウンロードするかどうかを決めています。

cocoa/Covid19Radar/Covid19Radar/Services/ExposureNotificationHandler.cs

Lines 234 to 274 in 3eac4f4

 Dictionary<string, long> lastTekTimestamp = userData.LastProcessTekTimestamp; 

 foreach (var tekItem in tekList) 

 { 

 long lastCreated = 0; 

 if (lastTekTimestamp.ContainsKey(region)) 

 { 

 lastCreated = lastTekTimestamp[region]; 

 } 

 else 

 { 

 lastTekTimestamp.Add(region, 0); 

 } 

 loggerService.Info($"tekItem.Created: {tekItem.Created}"); 

 if (tekItem.Created > lastCreated || lastCreated == 0) 

 { 

 var tmpFile = Path.Combine(tmpDir, Guid.NewGuid().ToString() + ".zip"); 

 Debug.WriteLine(Utils.SerializeToJson(tekItem)); 

 Debug.WriteLine(tmpFile); 

 loggerService.Info($"Download TEK file. url: {tekItem.Url}"); 

 using (Stream responseStream = await httpDataService.GetTemporaryExposureKey(tekItem.Url, cancellationToken)) 

 using (var fileStream = File.Create(tmpFile)) 

 { 

 try 

 { 

 await responseStream.CopyToAsync(fileStream, cancellationToken); 

 fileStream.Flush(); 

 } 

 catch (Exception ex) 

 { 

 loggerService.Exception("Fail to copy", ex); 

 } 

 } 

 lastTekTimestamp[region] = tekItem.Created; 

 downloadedFiles.Add(tmpFile); 

 Debug.WriteLine($"C19R FETCH DIAGKEY {tmpFile}"); 

 batchNumber++; 

 } 

 }

lastTekTimestamp[region]はダウンロード毎に更新されます。すべてのzipファイルがダウンロードされるとlastTekTimestampがuserData.LastProcessTekTimestampに反映され、直後にuserDataの保存処理が呼ばれます。

cocoa/Covid19Radar/Covid19Radar/Services/ExposureNotificationHandler.cs

Lines 276 to 277 in 3eac4f4

 userData.LastProcessTekTimestamp = lastTekTimestamp; 

 await userDataService.SetAsync(userData);

接触チェックはダウンロード後に行われるため、ダウンロードしかしていなくてもuserData.LastProcessTekTimestampが更新され、次回のダウンロードに参照されます。

チェックの対象になるのは直前のダウンロードされたzipファイルのみなので、未チェックのファイルを見つけてこれに加えるような仕組みがないのであれば、冒頭で挙げたところで中断した場合は、その分が対象になりません。

cocoa/Covid19Radar/Covid19Radar/Services/ExposureNotificationHandler.cs

Line 176 in 3eac4f4

await submitBatches(downloadedFiles);

C#のコーディング経験がないため勘違いもあるかもしれませんが、影響が大きいと思いましたので検証していただけると幸いです。

Internal IDs:

Bug 1518

The text was updated successfully, but these errors were encountered:

keiji · 2021-03-19T23:18:59Z

@yoshitomo-g 047b12b の変更で、すべてのTEKのダウンロード完了後に更新(266行目のexposureNotificationService.SetLastProcessTekTimestamp)に変わっていますね。

cocoa/Covid19Radar/Covid19Radar/Services/ExposureNotificationHandler.cs

Lines 231 to 272 in b823e87

 var lastCreated = exposureNotificationService.GetLastProcessTekTimestamp(region); 

 loggerService.Info($"lastCreated: {lastCreated}"); 

 var newCreated = lastCreated; 

 foreach (var tekItem in tekList) 

 { 

 loggerService.Info($"tekItem.Created: {tekItem.Created}"); 

 if (tekItem.Created > lastCreated || lastCreated == 0) 

 { 

 var tmpFile = Path.Combine(tmpDir, Guid.NewGuid().ToString() + ".zip"); 

 Debug.WriteLine(Utils.SerializeToJson(tekItem)); 

 Debug.WriteLine(tmpFile); 

 loggerService.Info($"Download TEK file. url: {tekItem.Url}"); 

 using (Stream responseStream = await httpDataService.GetTemporaryExposureKey(tekItem.Url, cancellationToken)) 

 using (var fileStream = File.Create(tmpFile)) 

 { 

 try 

 { 

 await responseStream.CopyToAsync(fileStream, cancellationToken); 

 fileStream.Flush(); 

 } 

 catch (Exception ex) 

 { 

 loggerService.Exception("Fail to copy", ex); 

 } 

 } 

 newCreated = tekItem.Created; 

 downloadedFiles.Add(tmpFile); 

 Debug.WriteLine($"C19R FETCH DIAGKEY {tmpFile}"); 

 batchNumber++; 

 } 

 } 

 loggerService.Info($"Batch number: {batchNumber}, Downloaded files: {downloadedFiles.Count()}"); 

 exposureNotificationService.SetLastProcessTekTimestamp(region, newCreated); 

 loggerService.Info($"region: {region}, newCreated: {newCreated}"); 

 loggerService.EndMethod(); 

 return (batchNumber, downloadedFiles); 

 }

問題なさそうならCloseするので、何かあればreopenお願いします。

yoshitomo-g · 2021-03-20T15:30:11Z

データの持ち方は変わりましたが、根本的なところでは変わっていないのではないでしょうか。

チェックまでのプロセスは次のように分けられると思うのですが、3で中断してしまった場合に、その続きから再開されるのではないというのが懸念事項です。1、3、2の順であれば良さそうですが。

ダウンロード
最後の tekItem.Created をアプリデータに反映
チェック

私の見落としているだけで解消できているようであれば、Closeで構いません。

yoshitomo-g · 2021-03-20T18:45:53Z

ここで LastProcessTekTimestamp 更新してしまっているのがタイミングとして早いと言う事でしょうか。
接触チェック前なのにチェックした事になってしまっている。
（コードを追い切れてないので私はまだ接触チェックのタイミングがどこか追い切れてません）

そうです。厳密に言えば、どこまでダウンロードしたかを管理して、チェックについてはダウンロードに依存している形ですね。
チェックは次のメソッドの第1引数を166行目でコールバックしていて、
（ダウンロードは154行目）

cocoa/Covid19Radar/Covid19Radar/Services/ExposureNotificationHandler.cs

Lines 129 to 194 in b823e87

 // this will be called when they keys need to be collected from the server 

 public async Task FetchExposureKeyBatchFilesFromServerAsync(Func<IEnumerable<string>, Task> submitBatches, CancellationToken cancellationToken) 

 { 

 var loggerService = LoggerService; 

 if (Interlocked.Exchange(ref fetchExposureKeysIsRunning, 1) == 1) 

 { 

 loggerService.Info("Skipped"); 

 return; 

 } 

 loggerService.StartMethod(); 

 try 

 { 

 // Migrate from UserData. 

 // Since it may be executed during the migration when the application starts, execute it here as well. 

 var userDataService = DependencyService.Resolve<IUserDataService>(); 

 await userDataService.Migrate(); 

 foreach (var serverRegion in AppSettings.Instance.SupportedRegions) 

 { 

 cancellationToken.ThrowIfCancellationRequested(); 

 loggerService.Info("Start download files"); 

 var (batchNumber, downloadedFiles) = await DownloadBatchAsync(serverRegion, cancellationToken); 

 loggerService.Info("End to download files"); 

 loggerService.Info($"Batch number: {batchNumber}, Downloaded files: {downloadedFiles.Count}"); 

 if (batchNumber == 0) 

 { 

 continue; 

 } 

 if (downloadedFiles.Count > 0) 

 { 

 loggerService.Info("C19R Submit Batches"); 

 await submitBatches(downloadedFiles); 

 // delete all temporary files 

 foreach (var file in downloadedFiles) 

 { 

 try 

 { 

 File.Delete(file); 

 } 

 catch (Exception ex) 

 { 

 // no-op 

 loggerService.Exception("Fail to delete downloaded files", ex); 

 } 

 } 

 } 

 } 

 } 

 catch (Exception ex) 

 { 

 // any expections, bail out and wait for the next time 

 loggerService.Exception("Fail to download files", ex); 

 } 

 finally 

 { 

 loggerService.EndMethod(); 

 Interlocked.Exchange(ref fetchExposureKeysIsRunning, 0); 

 } 

 }

メソッドの呼び出しとコールバックの中身は次のようになっています。

cocoa/Covid19Radar/Xamarin.ExposureNotification/ExposureNotification.shared.cs

Lines 88 to 120 in b823e87

 await Handler?.FetchExposureKeyBatchFilesFromServerAsync(async downloadedFiles => 

  { 

  cancellationToken.ThrowIfCancellationRequested(); 

  if (!downloadedFiles.Any()) 

  return; 

  if (nativeImplementation != null) 

  { 

  var r = await nativeImplementation.DetectExposuresAsync(downloadedFiles); 

  var hasMatches = (r.summary?.MatchedKeyCount ?? 0) > 0; 

  if (hasMatches) 

  await Handler.ExposureDetectedAsync(r.summary, r.getInfo); 

  } 

  else 

  { 

 #if __IOS__ 

  // On iOS we need to check this ourselves and invoke the handler 

  var (summary, info) = await PlatformDetectExposuresAsync(downloadedFiles, cancellationToken); 

  // Check that the summary has any matches before notifying the callback 

  if (summary?.MatchedKeyCount > 0) 

  await Handler.ExposureDetectedAsync(summary, info); 

 #elif __ANDROID__ 

  // on Android this will happen in the broadcast receiver 

  await PlatformDetectExposuresAsync(downloadedFiles, cancellationToken); 

 #endif 

  } 

  processedAnyFiles = true; 

  }, cancellationToken);

正常に動作していれば問題なさそうですが、中断された場合は前日24時間分として配信された TEK がチェックされないことになるかと。今のところ配信は1日に1回で、アプリ側では新規配信がないかを1日に複数回確認しています。

対案としては以下のどちらかになるでしょうか。

1. LastProcessTekTimestamp の更新を接触チェック後に移動する（できるかどうか？）
2. トランザクションの様な仕組みを利用してエラーが出たらデータを巻き戻す

1のほうが良い感じがしますが、LastProcessTekTimestamp に相当する値が途中までしか残らないので大変かもしれません。

検証できないのでアルゴリズムだけになりますが、簡易的なトランザクションとしてこのような方法はどうでしょうか。
チェックはチェックで別管理（仮に LastChekTekTimestamp とします）して、正常に完了したら LastProcessTekTimestamp をコピーします。ダウンロードの開始前に比較して一致していなかったらチェック未完了なので、ダウンロードの基準を LastProcessTekTimestamp ではなく LastChekTekTimestamp にします。TEK は外部にあるので、巻き戻すのではなく単純に再実行します。

keiji · 2021-03-21T04:47:33Z

@yoshitomo-g @b-wind

#68 の変更でどうですか？

DownloadBatchAsyncに引数でlastCreatedを与えて、戻り値でnewCreatedを受け取って、接触確認が終わった分のタイムスタンプを保存するパターンです。

UPDATE
説明が間違っていたので修正しました。

keiji · 2021-03-21T10:53:48Z

COCOA のメインページから「陽性者との接触を確認する」ボタンを押した後のページで表示されるのは
PreferenceKey.ExposureInformation に保存されてる値だと思うのですがどうでしょう？

その理解で正しいと思います。

#68 で変更された SetLastProcessTekTimestamp の実行は現状と同じく SetExposureInformation より前の為、
LastProcessTekTimestamp は更新されるが、ExposureInformation が更新されていないと言う状態があり得ます。
これは現象としては、接触の通知は行われるがアプリを開いても接触の履歴が見られないという事にならないでしょうか。

その事象は発生しないとぼくは考えています。

このIssueで指摘されている事象は「複数の診断キーをダウンロード中に、途中で通信が切断するなどの原因でダウンロードが中断し、再度ダウンロードした際に、前回の処理でダウンロードできてしまった診断キーについては『接触確認済』として取り扱われるので、本来されるべき接触確認が実行されない」と言うものと認識しています。

現在の処理ではLastProcessTekTimestampのタイムスタンプより後の診断キー（バッチ）をダウンロードするようになっていること。LastProcessTekTimestampの更新タイミングが「接触確認の実行」ではなく「診断キー（バッチ）のダウンロードが完了した直後」になっていることが原因と理解しています。

さて、ご指摘のSetExposureInformationの更新についてはExposureDetectedAsync 内で行われています。

cocoa/Covid19Radar/Covid19Radar/Services/ExposureNotificationHandler.cs

Lines 120 to 122 in b823e87

 loggerService.Info($"Save ExposureSummary. MatchedKeyCount: {userExposureSummary.MatchedKeyCount}"); 

 loggerService.Info($"Save ExposureInformation. Count: {exposureInformationList.Count}"); 

 exposureNotificationService.SetExposureInformation(userExposureSummary, exposureInformationList);

ExposureDetectedAsyncが呼び出されるのはFetchExposureKeyBatchFilesFromServerAsyncに与えているFunc<IEnumerable<string>, Task> submitBatchesの中です。

cocoa/Covid19Radar/Xamarin.ExposureNotification/ExposureNotification.shared.cs

Lines 88 to 120 in b823e87

 await Handler?.FetchExposureKeyBatchFilesFromServerAsync(async downloadedFiles => 

  { 

  cancellationToken.ThrowIfCancellationRequested(); 

  if (!downloadedFiles.Any()) 

  return; 

  if (nativeImplementation != null) 

  { 

  var r = await nativeImplementation.DetectExposuresAsync(downloadedFiles); 

  var hasMatches = (r.summary?.MatchedKeyCount ?? 0) > 0; 

  if (hasMatches) 

  await Handler.ExposureDetectedAsync(r.summary, r.getInfo); 

  } 

  else 

  { 

 #if __IOS__ 

  // On iOS we need to check this ourselves and invoke the handler 

  var (summary, info) = await PlatformDetectExposuresAsync(downloadedFiles, cancellationToken); 

  // Check that the summary has any matches before notifying the callback 

  if (summary?.MatchedKeyCount > 0) 

  await Handler.ExposureDetectedAsync(summary, info); 

 #elif __ANDROID__ 

  // on Android this will happen in the broadcast receiver 

  await PlatformDetectExposuresAsync(downloadedFiles, cancellationToken); 

 #endif 

  } 

  processedAnyFiles = true; 

  }, cancellationToken);

#68 の変更では、exposureNotificationService.SetLastProcessTekTimestampはsubmitBatchesより後に置いていますから、接触確認が終わった診断キー（バッチ）のlastCreatedが更新されていく

と理解しています。

UPDATE
AndroidではExposureNotificationCallbackService内で実行されていますね。

cocoa/Covid19Radar/Xamarin.ExposureNotification/CallbackService.android.cs

Lines 38 to 63 in b823e87

 class ExposureNotificationCallbackService : JobIntentService 

 { 

 const int jobId = 0x02; 

 public static void EnqueueWork(Context context, Intent work) 

 => EnqueueWork(context, Java.Lang.Class.FromType(typeof(ExposureNotificationCallbackService)), jobId, work); 

 protected override async void OnHandleWork(Intent workIntent) 

 { 

 Console.WriteLine($"C19R {nameof(ExposureNotificationCallbackService)}"); 

 var token = workIntent.GetStringExtra(ExposureNotificationClient.ExtraToken); 

 var summary = await ExposureNotification.PlatformGetExposureSummaryAsync(token); 

 Task<IEnumerable<ExposureInfo>> GetInfo() 

 { 

 return ExposureNotification.PlatformGetExposureInformationAsync(token); 

 } 

 // Invoke the custom implementation handler code with the summary info 

 Console.WriteLine($"C19R {nameof(ExposureNotificationCallbackService)}{summary?.MatchedKeyCount} Matched Key Count"); 

 if (summary?.MatchedKeyCount > 0) 

 { 

 await ExposureNotification.Handler.ExposureDetectedAsync(summary, GetInfo); 

 }

keiji · 2021-03-21T23:11:21Z

一晩考えてみたのですが、これは結構根の深い問題でXamarin.ENの変更が必要になりそうです。

接触確認に漏れが発生する可能性

「なにをもって接触確認を終えたと言うか」で対応が変わってくるのですが、
「submitBatchesを実行したら」という意味では #68 で足ります。

ただ、実際の接触確認はsubmitBatchesの中で行われていて、さらにAndroidとiOSで処理が異なります。

cocoa/Covid19Radar/Xamarin.ExposureNotification/ExposureNotification.shared.cs

Lines 88 to 123 in b823e87

 await Handler?.FetchExposureKeyBatchFilesFromServerAsync(async downloadedFiles => 

  { 

  cancellationToken.ThrowIfCancellationRequested(); 

  if (!downloadedFiles.Any()) 

  return; 

  if (nativeImplementation != null) 

  { 

  var r = await nativeImplementation.DetectExposuresAsync(downloadedFiles); 

  var hasMatches = (r.summary?.MatchedKeyCount ?? 0) > 0; 

  if (hasMatches) 

  await Handler.ExposureDetectedAsync(r.summary, r.getInfo); 

  } 

  else 

  { 

 #if __IOS__ 

  // On iOS we need to check this ourselves and invoke the handler 

  var (summary, info) = await PlatformDetectExposuresAsync(downloadedFiles, cancellationToken); 

  // Check that the summary has any matches before notifying the callback 

  if (summary?.MatchedKeyCount > 0) 

  await Handler.ExposureDetectedAsync(summary, info); 

 #elif __ANDROID__ 

  // on Android this will happen in the broadcast receiver 

  await PlatformDetectExposuresAsync(downloadedFiles, cancellationToken); 

 #endif 

  } 

  processedAnyFiles = true; 

  }, cancellationToken); 

 return processedAnyFiles; 

 }

iOSの方はPlatformDetectExposuresAsyncを呼んでから、接触があればExposureDetectedAsyncを呼ぶという構造です。つまり、submitBatchesが呼ばれたら接触確認が終わったと言えると考えます。

一方AndroidではPlatformDetectExposuresAsyncを呼ぶまでは同じですが、その結果はBroadcastRecieverで受け取るので、ここでExposureNotificationHandlerから離れてしまいます。

cocoa/Covid19Radar/Xamarin.ExposureNotification/CallbackService.android.cs

Lines 13 to 33 in b823e87

 [BroadcastReceiver( 

  Permission = "com.google.android.gms.nearby.exposurenotification.EXPOSURE_CALLBACK", 

  Exported = true)] 

 [IntentFilter(new[] { ExposureNotificationClient.ActionExposureStateUpdated, "com.google.android.gms.exposurenotification.ACTION_EXPOSURE_NOT_FOUND" })] 

 [Preserve] 

 class ExposureNotificationCallbackBroadcastReceiver : BroadcastReceiver 

 { 

 public override void OnReceive(Context context, Intent intent) 

 { 

 // https://developers.google.com/android/exposure-notifications/exposure-notifications-api#methods 

 var action = intent.Action; 

 if(action == ExposureNotificationClient.ActionExposureStateUpdated) 

 { 

 ExposureNotificationCallbackService.EnqueueWork(context, intent); 

 } 

 else if(action == "com.google.android.gms.exposurenotification.ACTION_EXPOSURE_NOT_FOUND") 

 { 

 Console.WriteLine($"C19R {nameof(ExposureNotificationCallbackBroadcastReceiver)} ACTION_EXPOSURE_NOT_FOUND."); 

 } 

 } 

 }

このBroadcastReceiverが呼ばれた時点が接触確認が終わったと言えます。
これを受け取らずにLastProcessTekTimestampが更新されてしまうと、万が一Google Play ServicesやEN APIに以上があって接触確認がうまくいかなかった場合に、取りこぼしが起きる可能性があります。本来は接触があって検査の必要なユーザーに通知が届かず、必要な検査や医療措置が遅れる可能性があります。
COCOAの目的と重要性を考えれば、ここは確実にケアしておくべきとぼくは考えています。

修正について

修正についてですが、問題は、このBroadcastReceiverがXamarin.ENの中にあることです。現時点でプロジェクトの中にあるので変更はできますが、将来のことを思えば触りたくないなという気持ちがあります。

代替案として考えたのは、ExposureNotificationCallbackBroadcastReceiverと同じIntent-Filterを持つBroadcastReceiverを作って、そこでも結果を受け取ることです。その場合、AttemptProcessTekTimestampのような中間値を作って取り回して、実際に処理が完了した時点でLastProcessTekTimestampに設定するという方法が考えられます。

この方法を採る上での課題は、iOSとAndroidでLastProcessTekTimestampの更新処理がぶつかると言うことです。iOSの方はsubmitBatchesの直後にLastProcessTekTimestampの更新を置くのが適切です。Androidでは、そこで更新してしまうとチェック漏れが発生する可能性があります。

iOS側ではXamarin.ENの外に「接触がなかった」と言うイベントを検出する手段が提供されていません（AndroidではACTION_EXPOSURE_NOT_FOUND ）。
これは最新のXamarin.ENでも同様なので、場合によってはPullRequestを送るなどする必要がありそうです（PullRequestを送った上で、暫定的にCOCOA内のXamarin.ENを変更するのは問題が起こりにくそう）。

ひとまず本IssueはPRも含めてCloseせず、検討を継続していきたいと考えています。

yoshitomo-g · 2021-03-22T10:15:58Z

見れていない間に話が大きくなっていて、今更になってしまうかもしれませんが・・・。

@b-wind

1のほうが良い感じがしますが、LastProcessTekTimestamp に相当する値が途中までしか残らないので大変かもしれません。

「途中までしか残らない」意味を取りかねました。

わかりにくい表現で失礼しました。変数としては DownloadBatchAsync の中でしか存在せず、メソッドを抜けた時点で消えてしまうという意味です。（#68 では引数の扱いを変えることでこれが解消されたことになります）

この値は複数の TEK をまとめたzipファイルのタイムスタンプで、DownloadBatchAsync の下記行でダウンロードする list.json での記述が大元です。中身は2次元配列のJSONです。

cocoa/Covid19Radar/Covid19Radar/Services/ExposureNotificationHandler.cs

Line 223 in b823e87

 List<TemporaryExposureKeyExportFileModel> tekList = await httpDataService.GetTemporaryExposureKeyList(region, cancellationToken); 

配列をループ処理しながら、zipファイルのダウンロード（L240~257）とタイムスタンプである newCreated の更新（L258)をして、ループ終了後に LastProcessTekTimestamp の更新に使われるのが最後です。v1.2.2の時点では、メソッド内で完結しています。

cocoa/Covid19Radar/Covid19Radar/Services/ExposureNotificationHandler.cs

Lines 231 to 267 in b823e87

 var lastCreated = exposureNotificationService.GetLastProcessTekTimestamp(region); 

 loggerService.Info($"lastCreated: {lastCreated}"); 

 var newCreated = lastCreated; 

 foreach (var tekItem in tekList) 

 { 

 loggerService.Info($"tekItem.Created: {tekItem.Created}"); 

 if (tekItem.Created > lastCreated || lastCreated == 0) 

 { 

 var tmpFile = Path.Combine(tmpDir, Guid.NewGuid().ToString() + ".zip"); 

 Debug.WriteLine(Utils.SerializeToJson(tekItem)); 

 Debug.WriteLine(tmpFile); 

 loggerService.Info($"Download TEK file. url: {tekItem.Url}"); 

 using (Stream responseStream = await httpDataService.GetTemporaryExposureKey(tekItem.Url, cancellationToken)) 

 using (var fileStream = File.Create(tmpFile)) 

 { 

 try 

 { 

 await responseStream.CopyToAsync(fileStream, cancellationToken); 

 fileStream.Flush(); 

 } 

 catch (Exception ex) 

 { 

 loggerService.Exception("Fail to copy", ex); 

 } 

 } 

 newCreated = tekItem.Created; 

 downloadedFiles.Add(tmpFile); 

 Debug.WriteLine($"C19R FETCH DIAGKEY {tmpFile}"); 

 batchNumber++; 

 } 

 } 

 loggerService.Info($"Batch number: {batchNumber}, Downloaded files: {downloadedFiles.Count()}"); 

 exposureNotificationService.SetLastProcessTekTimestamp(region, newCreated); 

 loggerService.Info($"region: {region}, newCreated: {newCreated}");

@keiji

このIssueで指摘されている事象は「複数の診断キーをダウンロード中に、途中で通信が切断するなどの原因でダウンロードが中断し、再度ダウンロードした際に、前回の処理でダウンロードできてしまった診断キーについては『接触確認済』として取り扱われるので、本来されるべき接触確認が実行されない」と言うものと認識しています。

現在の処理ではLastProcessTekTimestampのタイムスタンプより後の診断キー（バッチ）をダウンロードするようになっていること。LastProcessTekTimestampの更新タイミングが「接触確認の実行」ではなく「診断キー（バッチ）のダウンロードが完了した直後」になっていることが原因と理解しています。

私の認識と少し違うようなので、すり合わせをさせてください。

危惧しているタイミングは「複数の診断キーをダウンロード中に」ではなく、LastProcessTekTimestamp の更新後からチェック結果が ExposureInformation に反映されるまでの間です。LastProcessTekTimestamp の更新はダウンロードが終了した後ですので、DownloadBatchAsync のループ処理中であれば問題ないと考えています。原因のほうはそれで問題ありません。

tmurakami · 2021-03-22T11:37:27Z

#68 に先に書いてしまいましたが、私自身はsubmitBatches()の後にLastProcessTekTimestampを保存すれば十分なのではないかと考えています。
exposure-notifications-android (EN Expressのベース)でもprovideDiagnosisKeys()の後に最新TEKアーカイブパスを保存しているようなので。

EN に関する懸念事項を Google に確認することはできないのでしょうか？

ghost · 2021-03-22T14:17:10Z

わたしも Google に聞いたほうが早そうという気がしますが。

#68 (comment) で @tmurakami さんが参照されているのは
EN v2 (v1.5) 対応後の exposure-notifications-android のソースなのが若干気になります。

EN v1 時代の最後のものは 2020-06-26版 https://github.com/google/exposure-notifications-android/tree/1b215ca7fc99cac292b56a87751421bd1065df75 ですがそもそも downloadServerRepo にあたる
コードがなくて、TEKをどこまで処理したかの情報を取り扱っていなさそうな感じです(考慮されていない？)

v1/v2 など見比べてみての想像なのですが EN v1 が deprecated になった理由の一つが、#17 (comment) でも
指摘されている、「通知 (notification) とアプリ内のデータとが整合が取れなくなる場合がある」という
問題なのではないかと思えてきました。

つまり、Android 版の EN v1 では

provideDiagnosisKeys 呼び出し
broadcast 受信
getExposureSummary 呼び出し
getExposureInformation 呼び出し
戻り値をアプリ内表示用に保存

という処理順序で 4. まで実行して初めて通知(notification)が表示されるので、
@keiji さんの指摘通り、3. までで止まった場合は通知も表示されずアプリにも表示されない
パターンがあり得ますし、4. で止まった場合は通知は出たけれどアプリ内に表示されないという
可能性があると思います。なので、結局のところ 5. まで終わったところで処理完了とする
(5. まで完了しなかった場合は再度 1. からやり直すので二重通知されることがある) という実装にするのが安全そうです。

上記の EN v1 のころのサンプルでも token を全部呼び出し時に保存して

https://github.com/google/exposure-notifications-android/blob/1b215ca7fc99cac292b56a87751421bd1065df75/app/src/main/java/com/google/android/apps/exposurenotification/nearby/ProvideDiagnosisKeysWorker.java#L106

Receiver の処理が完了したら処理済みとしてmarkするような実装をしているようです

https://github.com/google/exposure-notifications-android/blob/1b215ca7fc99cac292b56a87751421bd1065df75/app/src/main/java/com/google/android/apps/exposurenotification/nearby/StateUpdatedWorker.java#L120

(mark されなかったのはどうするんだろうか？)

これをいくら厳密に直しても二重通知は回避できないので、EN v1.5 以降では token の概念を捨てて、
provideDiagnosisKeys が完了したらそこで atomic に処理完了とみなすことができて、
getExposureWindows は引数もないし何回呼び出しても同じ結果が常に返されるようになっているのではと推測します
(もしかしたら最後の provideDiagnosisKeys の呼び出しに対応する結果が返ってくるのかもですが
それはちょっと仕様としてはダメな気が。)

なので、諸々考え合わせると Google に問い合わせると EN v1.5 以降最新版にしてねという答えが返ってきてしまいそうな気がします。

あと、iOS 側の getExposureWindows は引数に ENExposureDetectionSummary を取る
(v1 の getExposureSummary と同じように detectExposures のコールバックから呼ばれる前提)
なので、そもそも同じようなメソッドだけど同じ結果が返ってくるのか(Xamarin.EN でちゃんと共通化できるのか) という
疑問が出てきますね。。

https://developer.apple.com/documentation/exposurenotification/enmanager/3644438-getexposurewindows

tmurakami · 2021-03-22T20:00:21Z

@zipperpull ご説明ありがとうございます。理解できました。

個人的には暫定で良いのでsubmitBatches()の後に更新するよう対応頂きたいと思っています。
そうすれば iOS は多分大丈夫でしょうし、Android もprovideDiagnosisKeys()まではケアできます。
もちろんそれ以上の対応がすぐにできるのであれば、それに越したことはないのですが...

今年 1月のprovideDiagnosisKeys()障害( corona-warn-app/cwa-app-android#2081 )のこともあり、本 issue の状況がとても気になっています。

ghost · 2021-03-22T20:34:08Z

わたしも @tmurakami さんや @yoshitomo-g さんがおっしゃるように

submitBatches()の後に更新するよう対応

ひとまずここまでを早めに(できるなら今回リリースで？)対応いただくのがよさそうと思いますが
難しそうでしょうか？

43275b8

だけマージする必要があるイメージで合っていますよね？

tmurakami · 2021-03-22T20:40:03Z

イメージで合っていますよね？

はい、そのイメージです。

keiji · 2021-03-22T21:20:06Z

ありがとうございます。寝て起きたらいろいろ情報が出ていて驚いています。

@yoshitomo-g

私の認識と少し違うようなので、すり合わせをさせてください。

ご指摘の通り「ダウンロード中断時に…」というのは、ぼくの認識に誤りがありました（PRを作っていて気づきました）。

@tmurakami @zipperpull

#68 についてはひとまず 43275b8 のみに変更する方向でいきます。EN APIに関してはGoogle（およびApple）から協力を得ることができるフローがあるので、引き続き調査を進めていきたいと考えています。

ぼくとしては昨日の朝に改善に着手した時に、修正自体は単純なものだろうと思っていたのですが、複雑な迷路に迷い込んだ感じです。

provideDiagnosisKeysのドキュメントを見ると、"After the result Task has returned, files can be deleted."とあって、ファイルを消すのは問題ないのだなと思いますが、やはり「接触確認が完了したタイミング」は確信が持てません。

https://developers.google.com/android/reference/com/google/android/gms/nearby/exposurenotification/ExposureNotificationClient?hl=en#provideDiagnosisKeys(com.google.android.gms.nearby.exposurenotification.DiagnosisKeyFileProvider)

corona-warn-app/cwa-app-android#2081 についても情報ありがとうございます。不勉強ながらぼく自身がこの事例を知らなかったので、とても勉強になります。

ghost · 2021-03-23T00:04:44Z

一応イギリスの issue はこちらです
ukhsa-collaboration/covid-19-app-android-ag-public#39

この #17 とは別問題じゃないかとは思いますが。。(原因不明なのでわかりませんけど)

ghost · 2021-03-23T01:07:00Z

Google の provideDiagnosisKeys の仕様だと Task が返されるので、broadcast 送信までは完了した時点で
Task は Success 状態にはなるのかなとは思えますね(仕様にはそこまで明記されていないけど)

https://developers.google.com/android/reference/com/google/android/gms/nearby/exposurenotification/ExposureNotificationClient?hl=en#public-abstract-taskvoid-providediagnosiskeys-listfile-keyfiles,-exposureconfiguration-configuration,-string-token

もしそうなると仮定すると、呼び出し前に token を保存して、Success した token のぶんは
フラグを立てておけば、14日以内ならあとから(broadcast の文脈以外でも)
getExposureSummary を呼べばデータは取り出せて通知もそこで出す(#17 (comment) でいうと 3以降を後からやる)
仕組みを追加することはできそうかなと思いました。それができる仕組みであれば、
Task が Success したらファイルを消してもよい、という仕様と矛盾はなさそうです。
(ほとんど推測なので万が一やるとしても Google に聞いてみてからで)

追記: 保存まで終わったらそれは別のフラグ (mark 相当) をたてる必要もありますね(複雑)
追記2: もしかして StateUpdatedWorker は enqueue しているので、途中でタスクキルなどされた場合も 3以降を後でやる、も意図として自然に含まれているという可能性があるのですね。。

keiji · 2021-03-23T01:14:01Z

今のXamarin.ENだとtokenはXamarin.ENの中で生成しているので、そこを変えないといけませんね。
フラグ管理とか入ってくるとDatabaseで持った方がいい気がしますが……ひとまずステータスをwaiting-for-confirmationにして、進展があれば共有しますね。

keiji · 2021-06-14T01:01:32Z

#68 の変更が効果があることを確認できました。

https://twitter.com/h_okumura/status/1404221661591658499

detectExposureをしたときにENErrorDomain Code=16 "(null)" UserInfo={cuErrorMsg=Database inaccessible}が返ってくるケースが見つかりました。
v1.2.2以前ではLastProcessTekTimestampが更新されているので取りこぼしが発生しますが、LastProcessTekTimestampの書き換えタイミングを変更した、Android版のv1.2.3、ならびにiOS版のv1.2.4では修正されているという認識です。

b-wind · 2021-06-15T02:01:29Z

無事 iOS 版でもリリースされ、効果も確認されたようでホッとしています。
提案ですが、この Issue は一端ここで close してしまい、より詳細についての対応は新規 Issue で対応というわけには行かないでしょうか。

理由は単純で、後から参加した人がリリースとIssueの対応を見るときに分かりにくくなりそうだと思ったからです。
この Issue は暫定対応、新しいIssueは根本対応という分け方であれば混乱も少ないと思うのですが。

yoshitomo-g · 2021-06-15T10:14:31Z

提案ですが、この Issue は一端ここで close してしまい、より詳細についての対応は新規 Issue で対応というわけには行かないでしょうか。

そもそもの懸念していたことが解消されたということであれば、私は close で良いと思います。#17 (comment) で暫定対応という方針が出ていますし。
（ #17 (comment) のエラーは、submitBatches() の中でロックされているファイルを更新しようとして中断したのだろうというくらいにしかわかっていません。）

keiji · 2021-07-13T16:10:24Z

@yoshitomo-g それでは本IssueはCloseしますね。

keiji self-assigned this Mar 19, 2021

keiji mentioned this issue Mar 21, 2021

Fix the issue that the timing of saving lastCreated is inappropriate #68

Closed

keiji added the bug バグ。本来あるべき動作をしていないもの label Mar 22, 2021

keiji added the waiting-for-confirmation 関係者に確認中のもの label Mar 23, 2021

heykuro mentioned this issue Apr 6, 2021

Issue と PR の優先順位を明記する（マイルストーンまたはプロジェクトボードを利用する） #102

Closed

keiji added the ready-for-release マージが済み、リリース準備が完了しているもの label Apr 8, 2021

cocoa-dev mentioned this issue Apr 21, 2021

Release v1.2.3 #122

Merged

tatsu-jp mentioned this issue May 15, 2021

Debug_Mockの機能拡張（高リスク接触等） #179

Closed

kvaluation mentioned this issue May 21, 2021

接触確認の意図しない挙動について情報収集をする準備 #157

Open

b-wind mentioned this issue Jun 13, 2021

ETag による通信量削減 #199 の解決 #233

Draft

cocoa-dev added the confirmed 開発内部管理用 label Jul 1, 2021

keiji added released リリースが完了したもの and removed waiting-for-confirmation 関係者に確認中のもの ready-for-release マージが済み、リリース準備が完了しているもの labels Jul 8, 2021

keiji closed this as completed Jul 13, 2021

tmurakami mentioned this issue Aug 4, 2021

ExposureNotificationバインディングを最新にする #278

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

接触チェックに漏れが発生する可能性について #17

接触チェックに漏れが発生する可能性について #17

yoshitomo-g commented Jan 20, 2021 •

edited by cocoa-dev

Loading

keiji commented Mar 19, 2021 •

edited

Loading

yoshitomo-g commented Mar 20, 2021

yoshitomo-g commented Mar 20, 2021

keiji commented Mar 21, 2021 •

edited

Loading

keiji commented Mar 21, 2021 •

edited

Loading

keiji commented Mar 21, 2021

yoshitomo-g commented Mar 22, 2021

tmurakami commented Mar 22, 2021

ghost commented Mar 22, 2021

tmurakami commented Mar 22, 2021

ghost commented Mar 22, 2021

tmurakami commented Mar 22, 2021

keiji commented Mar 22, 2021

ghost commented Mar 23, 2021

ghost commented Mar 23, 2021 •

edited by ghost

Loading

keiji commented Mar 23, 2021 •

edited

Loading

keiji commented Jun 14, 2021

b-wind commented Jun 15, 2021 •

edited

Loading

yoshitomo-g commented Jun 15, 2021

keiji commented Jul 13, 2021

接触チェックに漏れが発生する可能性について #17

接触チェックに漏れが発生する可能性について #17

Comments

yoshitomo-g commented Jan 20, 2021 • edited by cocoa-dev Loading

keiji commented Mar 19, 2021 • edited Loading

yoshitomo-g commented Mar 20, 2021

yoshitomo-g commented Mar 20, 2021

keiji commented Mar 21, 2021 • edited Loading

keiji commented Mar 21, 2021 • edited Loading

keiji commented Mar 21, 2021

接触確認に漏れが発生する可能性

修正について

yoshitomo-g commented Mar 22, 2021

tmurakami commented Mar 22, 2021

ghost commented Mar 22, 2021

tmurakami commented Mar 22, 2021

ghost commented Mar 22, 2021

tmurakami commented Mar 22, 2021

keiji commented Mar 22, 2021

ghost commented Mar 23, 2021

ghost commented Mar 23, 2021 • edited by ghost Loading

keiji commented Mar 23, 2021 • edited Loading

keiji commented Jun 14, 2021

b-wind commented Jun 15, 2021 • edited Loading

yoshitomo-g commented Jun 15, 2021

keiji commented Jul 13, 2021

yoshitomo-g commented Jan 20, 2021 •

edited by cocoa-dev

Loading

keiji commented Mar 19, 2021 •

edited

Loading

keiji commented Mar 21, 2021 •

edited

Loading

keiji commented Mar 21, 2021 •

edited

Loading

ghost commented Mar 23, 2021 •

edited by ghost

Loading

keiji commented Mar 23, 2021 •

edited

Loading

b-wind commented Jun 15, 2021 •

edited

Loading