-
Notifications
You must be signed in to change notification settings - Fork 604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(filesystem): web appendFile with base64 data #928
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the code should behave more like on iOS/Android where instead of checking if the existing content is valid base64, it checks if the encoding option is passed.
Then, if no encoding is passed it means it should be base64, so should validate that it's true (for writeFile too).
At the moment we don't validate and allow to write without encoding on web while on iOS/Android it fails. If we validate before writing we don't need to validate the existing value.
filesystem/src/web.ts
Outdated
@@ -240,7 +247,13 @@ export class FilesystemWeb extends WebPlugin implements FilesystemPlugin { | |||
} | |||
|
|||
if (occupiedEntry !== undefined) { | |||
data = occupiedEntry.content + data; | |||
if (occupiedEntry.content && !encoding) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible that occupiedEntry.content is undefined?
If it's not possible then we shouldn't check
If it's possible then there could be a case where there is no content and no encoding and it goes to the else and allows to write base64 without checking if it's base64
filesystem/src/web.ts
Outdated
@@ -240,7 +247,13 @@ export class FilesystemWeb extends WebPlugin implements FilesystemPlugin { | |||
} | |||
|
|||
if (occupiedEntry !== undefined) { | |||
data = occupiedEntry.content + data; | |||
if (occupiedEntry.content && !encoding) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible that occupiedEntry.content is undefined?
If it's not possible then we shouldn't check
If it's possible then there could be a case where there is no content and no encoding and it goes to the else and allows to write base64 without checking if it's base64
fixes: #899