Skip to content

Commit

Permalink
fixes #10. uri decodes the attachments path to account for potential …
Browse files Browse the repository at this point in the history 
…platform encodings.
  • Loading branch information
@JoshJuncker
JoshJuncker committed Jun 10, 2022
1 parent f80e751 commit 6d3298c
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion share_handler_platform_interface/lib/src/data/messages.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class SharedAttachment {
static SharedAttachment decode(Object message) {
final Map<Object?, Object?> pigeonMap = message as Map<Object?, Object?>;
return SharedAttachment(
path: pigeonMap['path']! as String,
path: Uri.decodeFull(pigeonMap['path']! as String),
type: SharedAttachmentType.values[pigeonMap['type']! as int],
);
}
Expand Down

12 comments on commit 6d3298c

@MagTuxGit
Copy link
Contributor

@MagTuxGit MagTuxGit commented on 6d3298c Jul 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes exception on Android when sharing file from GDrive with Cyrillic letters in the path.

void main() {
  const filePath = "/data/user/0/app.example/cache/путін - вбивця.pdf";
  final decodedPath = Uri.decodeFull(filePath);
  print(decodedPath);
}

Uncaught Error: Invalid argument(s): Illegal percent encoding in URI

@czredhat
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MagTuxGit is right, please remove Uri.decodeFull

@JoshJuncker
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@czredhat What would you prefer instead? A try catch that tries to decodeFull, but the catch defaults to the raw value?

@czredhat
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @JoshJuncker , thank for your reply. I have just sent a new pull request #39 . I have to done this update because many files were not working. Files with diacritics in the filename for example. The code in the pull request uses the raw value and it is fully working. I have the code already in my own app and there is no problem. All files are working. share_handler is great library! thank you.

@JoshJuncker
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@czredhat I hesitate to remove the Uri.decodeFull as it was added purposefully to solve an issue with encoded urls. I think I will use a try catch instead so as not to lose that functionality.

@czredhat
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JoshJuncker I think It coudnt work in both the ways because for example this úpěl ódy$&amp;=%20červený kůň žluťoučký řňě.gif is valid file name and it contains & amp; but it is not an entity.
Could you give me an example when there is an encoded url please? Maybe I dont understand fully to all possible scenarios. Thank you

@JoshJuncker
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@czredhat I believe that sometimes (maybe only iOS?) it returns the url in an encoded format with spaces encoded as %20 etc. The url is unusable in that format. It has to be decoded. I previous false assumption is that I could call decodeFull on any valid url, and it would work. Apparently that is not the case. So putting it in a try catch may be a better idea to still handle the case when an encoded url is returned from the native code.

@czredhat
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JoshJuncker I dont think so. Try + Catch is not a solution.
Try to rename a file to this name aaa%20bbb.gif; it is a valid name but Uri.decodeFull( will translate the name to aaa bbb.gif without any error and it is the problem of the solution with Try Catch.
Btw. I tried it I renamed a file to this name on my iphone and the code without Uri.decodeFull( is working well and the file was imported. I tried many various combination. Local files, files in iCloud, with special characters ... a lot of combination... and everything is working. On Android it is working as well. I have my app with this update in production and everything looks okey. At least it is working much much better than with Uri.decodeFull( because Uri.decodeFull( doesn't work with many standard files.

@czredhat
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry @JoshJuncker maybe I have the error because your Uri.decodeFull( colides with another Uri.decodeFull( in my own code. So I have to do more investigation and maybe your code is right. I will message you later. sorry

@czredhat
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JoshJuncker Hi Josh, I am sorry. I had a bug in my code and how I said before, it colided with your Uri.decodeFull(. So here is the new result from my investigation...

You are right. Uri.decodeFull( is really needed on iOS.
But it causes problems on Android. Files with diacritics or special characters are not working on Android when there is the Uri.decodeFull(.

So... I have just updated my pull request. It is still a very small update, the only change is if (Platform.isIOS) { ...

I have tested it on iOS and on Android as well. Now I am sure. Please look on my pull request #39 and consider it.

And please remove this older pull request #21 because its way is not working (I have tested it).

Thank you very much a have a nice day.

@JoshJuncker
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@czredhat Did you happen to try this other solution in this pr? https://github.com/ShoutSocial/share_handler/pull/21/files

@czredhat
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JoshJuncker Yes I did. and it doesnt work well on Android.

Please sign in to comment.