cordova-plugin-file
cordova-plugin-file copied to clipboard
fix(android, ios, windows) Handle multi-byte UTF-8 characters that cross a chunk boundary (CB-13570)
Platforms affected
Android, iOS, Windows.
My organization hit this one in production in a highly visible (to the end user) setting. So I've gone ahead and fixed the 3 platforms we target. I haven't yet tested on OSX or browser, but looking at the source it looks like fixes are likely needed on those platforms too. I don't think that needs to hold up this PR, b/c I've made my change in a backwards compatible way. But that could be an area to discuss.
What does this PR do?
Fixes CB-13570 on the specified platforms. Specifically, this PR changes the JS-to-native interface for the readAsX methods. Previously the JS side expected the native side to return the read value (be it a text string, ArrayBuffer, data URL, etc.) With this PR, the JS side now can handle two result formats from the native side:
- An object like
{ value: any, numBytesConsumed: number }, wherevalueis the read value (text, ArrayBuffer, etc.) andnumBytesConsumedis the number of bytes consumed to read that value.numBytesConsumedcan differ from the specifiedREAD_CHUNK_SIZEfor reasons that will be clear shortly. - The previous format, for backwards compatibility with platforms/read methods that haven't yet had their native sides updated for this change.
Then, on Android, iOS, and Windows, the native side uses this new flexibility to change its handling for readAsText specifically. Depending on the specified encoding, if the end offset requested by the JS side would cause a multi-byte character to get split, the native side extends the end offset as needed to prevent splitting. The native side then returns an accurate numBytesConsumed to reflect the extra bytes needed.
What testing has been done on this change?
I added an automated test that exposes the bug and now passes on the fixed platforms. I also did manual testing in the app that exposed the bug for us.
Checklist
- [x] Reported an issue in the JIRA database
- [x] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
- [x] Added automated test coverage as appropriate for this change.
UPDATE: It looks like CI has confirmed that this bug is indeed present on the browser platform, due to the new test I added. Again, from my point of view, I don't think that needs to hold up this PR, b/c no existing functionality was regressed. But I'm happy to discuss more.
Thanks!
UPDATE: I had inadvertently broken compatibility w/ Java < 8 on Android. Just updated with a fix.
Rerun the tests with current configuration
CI: Once more please.
The new file.spec.84.2 is failing for all browser runs:
**************************************************
* Failures *
**************************************************
1) cordova-plugin-file-tests.tests >> File API Read method file.spec.84.2 should read multi-byte UTF-8 chars across chunk boundaries
- Expected '---��---ࠀ--ࠀ---����--��-���' to equal '------ࠀ--ࠀ---𐀀--𐀀-𐀀'.