----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12548/#review23273 -----------------------------------------------------------
framework/src/org/apache/cordova/Config.java <https://reviews.apache.org/r/12548/#comment47158> Add data:* to this list. framework/src/org/apache/cordova/Whitelist.java <https://reviews.apache.org/r/12548/#comment47160> make private & static? framework/src/org/apache/cordova/Whitelist.java <https://reviews.apache.org/r/12548/#comment47162> Allow -> allow framework/src/org/apache/cordova/Whitelist.java <https://reviews.apache.org/r/12548/#comment47168> Instead of parsing the url yourself, it might be better to use: Uri.parse(url) and use: getScheme(), getHost(), getPort(), getPath(). Probably even better would be to take in a Uri as a parameter both here and in isUrlWhitelisted. That way it's not re-parsed for each pattern. framework/src/org/apache/cordova/Whitelist.java <https://reviews.apache.org/r/12548/#comment47167> Doesn't look like this cache is very good. - It's unbounded - It's caching URLs without stripping query params - It's failing to cache negative hits. I don't think performance is an issue here, so probably a good idea to just delete the cache. Sorry, didn't see this earlier. - Andrew Grieve On July 15, 2013, 2:57 p.m., Ian Clelland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12548/ > ----------------------------------------------------------- > > (Updated July 15, 2013, 2:57 p.m.) > > > Review request for cordova. > > > Repository: cordova-android-git > > > Description > ------- > > Replaced the current android whitelist implementation with a new one which > conforms to CB-4093. > > This is also more robust against bad urls in the config file, and handles > several cases which the previous implementation did not (eg. > http://www.apache.org:passw...@evil.com/ would be accepted previously) > > > Diffs > ----- > > framework/src/org/apache/cordova/Config.java 6e0c147 > framework/src/org/apache/cordova/Whitelist.java 736e5a7 > > Diff: https://reviews.apache.org/r/12548/diff/ > > > Testing > ------- > > This passes all of the recently-added whitelist tests in cordova-mobile-spec. > > > Thanks, > > Ian Clelland > >
