alwx / react-native-photo-view

Pinch-to-zoom view for React Native (both iOS and Android)
MIT License
816 stars 434 forks source link

iOS: Fix special character issue in image URL. #157

Open gokcan opened 6 years ago

gokcan commented 6 years ago

Hello @alwx,

If the external URL contains special characters such as 你好 and مرحبا, image could not be displayed. We have faced this issue in https://github.com/zulip/zulip-mobile/issues/2113.

To resolve this, we need to escape special characters in uri using UTF-8 encoding.

Fixes #129

gnprice commented 6 years ago

I think this is not a correct fix -- it risks double-encoding. For example, if uri is a properly percent-encoded URL like https://commons.wikimedia.org/wiki/File:Mezquita_de_Nasirolmolk,_Shiraz,_Ir%C3%A1n,_2016-09-24,_DD_60-62_HDR.jpg, we would make a request for https://commons.wikimedia.org/wiki/File:Mezquita_de_Nasirolmolk,_Shiraz,_Ir%25C3%25A1n,_2016-09-24,_DD_60-62_HDR.jpg, which does not work.

Inside RN itself, RCTConvert has some fallback logic to try to accept both proper URLs and proto-URLs containing arbitrary characters that need to be percent-encoded. It looks like this: https://github.com/facebook/react-native/blob/f8d6b97140cffe8d18b2558f94570c8d1b410d5c/React/Base/RCTConvert.m#L83-L97

I think the ideal fix would be to use RCTConvert itself, if that's possible, in order to support exactly the same set of uri values as RN's own Image component. See the original report of this issue, #129, which observes that non-ASCII characters work fine in RN's Image.

If that's not possible to use directly (I don't know enough about the internals of a library like this to know if it is), probably the next-best is to copy-paste that bit of logic from RCTConvert.