Consider data: URLs local in KIO::AccessManager

Review Request #101140 - Created April 16, 2011 and submitted

Information
Volker Krause
kdelibs
master
Reviewers
kdelibs
Currently KIO::AccessManager blocks retrieval of embedded data: URLs if external references are disabled. This does not match the behavior in KHTML and breaks for example the display of sender photos/logos in KMail (which uses kdewebkit).

  
Kevin Krammer
Wouldn't it make more sense to change KProtocolInfo::protocolClass() such that it considers data: to be local access?
  1. That was indeed my first attempt, but David pointed out that this would have further (security) implications, since the protocol class is also used in a number of different places where the use of data: might not be desired. Instead, I followed the approach chosen by KHTML now, explicitly white-listing data: only for retrieval but nothing else.
  2. Well I guess I am the one that broke this in an attempt to make it more generic. The change seems fine, but you patch should then do the following:
    
        if (scheme.compare(QL1S("data"), Qt::CaseInsensitive) == 0)
            return true;
    
        if (KProtocolInfo::isKnownProtocol(scheme) &&
            KProtocolInfo::protocolClass(scheme).compare(QL1S(":local"), Qt::CaseInsensitive) == 0)
            return true;
    
        return false;
    
    since isLocalRequest is more likely to encounter the "data" protocol than any other local protocol.
  3. The case-insensitive comparisson seems unnecessary, KUrl::protocol() already returns a lower-cased string. 
    
    Regarding the reordering, isn't file: the most likely local protocol? I don't know where else this is used, but in KMail it definitely is much more common than data:.
Dawit Alemayehu
Hmm... did not know KUrl::protocol() already returned a lower case text. And it is probably true the file protocol might be more previlant when access to external content is disallowed. Well then go ahead... Do not forget to backport or forward port depending on how you work :)
Commit Hook
This review has been submitted with commit a3297d274843c22ee8f5c4ede64f9c62311ade37 by Volker Krause.
Commit Hook
This review has been submitted with commit be0cc2198d3c595697053a7a947c200aa7e9d9a7 by Volker Krause.
Loading...