Removed static factories for data, ftp, file, and about jobs.

net/url_request/url_request_http_job.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/url_request/url_request_http_job.h"
 
 #include "base/base_switches.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
@@ skipping 25 matching lines @@
 #include "net/http/http_status_code.h"
 #include "net/http/http_transaction.h"
 #include "net/http/http_transaction_delegate.h"
 #include "net/http/http_transaction_factory.h"
 #include "net/http/http_util.h"
 #include "net/url_request/fraudulent_certificate_reporter.h"
 #include "net/url_request/http_user_agent_settings.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_error_job.h"
+#include "net/url_request/url_request_job_factory.h"
 #include "net/url_request/url_request_redirect_job.h"
 #include "net/url_request/url_request_throttler_header_adapter.h"
 #include "net/url_request/url_request_throttler_manager.h"
 
 static const char kAvailDictionaryHeader[] = "Avail-Dictionary";
 
 namespace net {
 
 class URLRequestHttpJob::HttpFilterContext : public FilterContext {
  public:
@@ skipping 954 matching lines @@
 
   return !encoding_types.empty()
       ? Filter::Factory(encoding_types, *filter_context_) : NULL;
 }
 
 bool URLRequestHttpJob::IsSafeRedirect(const GURL& location) {
   // We only allow redirects to certain "safe" protocols.  This does not
   // restrict redirects to externally handled protocols.  Our consumer would
   // need to take care of those.
 
-  if (!URLRequest::IsHandledURL(location))
+  if (!URLRequest::IsHandledURL(location) &&
+      !request_->context()->job_factory()->IsHandledURL(location)) {

[mmenke, 2013/01/22 16:50:37]

I believe this may change behavior.  In particular, I believe we allow redirects
to chrome-app:// URLs (Or something like that.  Can't remember the exact
scheme).  If we have ProtocolHandlers for them, we'll now return false here.

[pauljensen, 2013/01/23 21:43:33]

What makes a redirect "unsafe"?  The only description I see is at the definition
of ERR_UNSAFE_REDIRECT where we mention file:// being unsafe.

The comment 2 lines above this says we should allow redirects to "externally
handled protocols".  Does this mean things registered with
navigator.registerProtocolHandler()?  In which case I think my change would also
change/break this behavior.

The chrome-app:// CL never landed: http://crrev.com/11519027
chrome-app:// was added to
ChildProcessSecurityPolicyImpl::ChildProcessSecurityPolicyImpl()'s list of
explicitly safe schemes, so maybe it should be added here to kSafeSchemes as
part of http://crrev.com/11519027

[mmenke, 2013/01/23 22:09:17]

I believe it encompasses everything we don't want websites to be able to
redirect to that's also handled by the network stack.  Other protocols are
supposed to be handled elsewhere.

One option would be to add an IsSafeRedirect function to ProtocolHandlers, and
call the function of the protocol handler, if there is one.  If there isn't,
return true.

Default for a ProtocolHandler would be to return true (Again, with the
assumption that things outside of net are handled by logic elsewhere).  Long
term, think it would be better for them return false by default.

Not sure that this is ideal behavior, just a way to maintain current behavior,
which seems to be kind of a mess.  I suspect it's primarily intended for non-RDH
requests and external consumers of our network stack, but I'm honestly not sure.


Adding chrome-app as a safe scheme here would be a laying violation.

[pauljensen, 2013/01/28 19:51:49]

Why wouldn't we want websites to redirect to something?

     return true;
+  }
 
   static const char* kSafeSchemes[] = {
     "http",
     "https",
     "ftp"
   };
 
   for (size_t i = 0; i < arraysize(kSafeSchemes); ++i) {
     if (location.SchemeIs(kSafeSchemes[i]))
       return true;
@@ skipping 538 matching lines @@
 
 void URLRequestHttpJob::NotifyURLRequestDestroyed() {
   awaiting_callback_ = false;
 }
 
 void URLRequestHttpJob::OnDetachRequest() {
   http_transaction_delegate_->OnDetachRequest();
 }
 
 }  // namespace net