From 749b68b70f15c5643b8bbc622c8dc5d5870687bb Mon Sep 17 00:00:00 2001 From: "tyoshino@chromium.org" Date: Fri, 26 Jul 2013 10:54:37 +0000 Subject: Remove security_origin member from content::Stream. Stream::security_origin_ is not used at all for now. There's no established code in Chromium to do origin checking. Security of Blob/Stream resource loading now depends on origin checking mechanism in Blink. Keeping this variable without having any checking mechanism confuses developers. Remove it until we build origin checking code in Chromium. Also, rename security_origin to origin, as it's nothing but an origin URL. BUG=263342 Review URL: https://chromiumcodereview.appspot.com/19798012 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@213830 0039d316-1c4b-4281-b951-d872f2087c98 --- .../browser/loader/resource_dispatcher_host_impl.cc | 9 ++++----- content/browser/loader/stream_resource_handler.cc | 10 +++++----- content/browser/loader/stream_resource_handler.h | 6 ++++-- content/browser/streams/stream.cc | 3 --- content/browser/streams/stream.h | 10 +++++----- content/browser/streams/stream_unittest.cc | 18 +++++++++--------- .../browser/streams/stream_url_request_job_unittest.cc | 8 ++++---- 7 files changed, 31 insertions(+), 33 deletions(-) (limited to 'content/browser') diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc index f21f290..84bb203 100644 --- a/content/browser/loader/resource_dispatcher_host_impl.cc +++ b/content/browser/loader/resource_dispatcher_host_impl.cc @@ -567,13 +567,13 @@ ResourceDispatcherHostImpl::MaybeInterceptAsStream(net::URLRequest* request, ResourceRequestInfoImpl* info = ResourceRequestInfoImpl::ForRequest(request); const std::string& mime_type = response->head.mime_type; - GURL security_origin; + GURL origin; std::string target_id; if (!delegate_ || !delegate_->ShouldInterceptResourceAsStream(info->GetContext(), request->url(), mime_type, - &security_origin, + &origin, &target_id)) { return scoped_ptr(); } @@ -581,11 +581,10 @@ ResourceDispatcherHostImpl::MaybeInterceptAsStream(net::URLRequest* request, StreamContext* stream_context = GetStreamContextForResourceContext(info->GetContext()); - scoped_ptr handler( new StreamResourceHandler(request, stream_context->registry(), - security_origin)); + origin)); info->set_is_stream(true); delegate_->OnStreamCreated( @@ -595,7 +594,7 @@ ResourceDispatcherHostImpl::MaybeInterceptAsStream(net::URLRequest* request, target_id, handler->stream()->CreateHandle(request->url(), mime_type), request->GetExpectedContentSize()); - return (scoped_ptr(handler.release())).Pass(); + return handler.PassAs(); } void ResourceDispatcherHostImpl::ClearSSLClientAuthHandlerForRequest( diff --git a/content/browser/loader/stream_resource_handler.cc b/content/browser/loader/stream_resource_handler.cc index c832afa..1e2acc8 100644 --- a/content/browser/loader/stream_resource_handler.cc +++ b/content/browser/loader/stream_resource_handler.cc @@ -18,13 +18,14 @@ namespace content { StreamResourceHandler::StreamResourceHandler( net::URLRequest* request, StreamRegistry* registry, - const GURL& security_origin) + const GURL& origin) : request_(request), read_buffer_(NULL) { - // TODO(zork): Find a way to share this with the blob URL creation in WebKit. + // TODO(tyoshino): Find a way to share this with the blob URL creation in + // WebKit. GURL url(std::string(chrome::kBlobScheme) + ":" + - security_origin.spec() + base::GenerateGUID()); - stream_ = new Stream(registry, this, security_origin, url); + origin.spec() + base::GenerateGUID()); + stream_ = new Stream(registry, this, url); } StreamResourceHandler::~StreamResourceHandler() { @@ -115,4 +116,3 @@ void StreamResourceHandler::OnClose(Stream* stream) { } } // namespace content - diff --git a/content/browser/loader/stream_resource_handler.h b/content/browser/loader/stream_resource_handler.h index ce540578..037ebdc 100644 --- a/content/browser/loader/stream_resource_handler.h +++ b/content/browser/loader/stream_resource_handler.h @@ -23,9 +23,12 @@ class StreamRegistry; class StreamResourceHandler : public StreamWriteObserver, public ResourceHandler { public: + // |origin| will be used to construct the URL for the Stream. See + // WebCore::BlobURL and and WebCore::SecurityOrigin in Blink to understand + // how origin check is done on resource loading. StreamResourceHandler(net::URLRequest* request, StreamRegistry* registry, - const GURL& security_origin); + const GURL& origin); virtual ~StreamResourceHandler(); virtual bool OnUploadProgress(int request_id, @@ -78,4 +81,3 @@ class StreamResourceHandler : public StreamWriteObserver, } // namespace content #endif // CONTENT_BROWSER_LOADER_STREAM_RESOURCE_HANDLER_H_ - diff --git a/content/browser/streams/stream.cc b/content/browser/streams/stream.cc index 27244e1..38f91b82 100644 --- a/content/browser/streams/stream.cc +++ b/content/browser/streams/stream.cc @@ -22,11 +22,9 @@ namespace content { Stream::Stream(StreamRegistry* registry, StreamWriteObserver* write_observer, - const GURL& security_origin, const GURL& url) : data_bytes_read_(0), can_add_data_(true), - security_origin_(security_origin), url_(url), data_length_(0), registry_(registry), @@ -147,4 +145,3 @@ void Stream::OnDataAvailable() { } } // namespace content - diff --git a/content/browser/streams/stream.h b/content/browser/streams/stream.h index 912e437..4dd852f1a 100644 --- a/content/browser/streams/stream.h +++ b/content/browser/streams/stream.h @@ -38,10 +38,13 @@ class CONTENT_EXPORT Stream : public base::RefCountedThreadSafe { STREAM_EMPTY, }; - // Creates a stream useable from the |security_origin|. + // Creates a stream. + // + // Security origin of Streams is checked in Blink (See BlobRegistry, + // BlobURL and SecurityOrigin to understand how it works). There's no security + // origin check in Chromium side for now. Stream(StreamRegistry* registry, StreamWriteObserver* write_observer, - const GURL& security_origin, const GURL& url); // Sets the reader of this stream. Returns true on success, or false if there @@ -75,8 +78,6 @@ class CONTENT_EXPORT Stream : public base::RefCountedThreadSafe { const GURL& url() const { return url_; } - const GURL& security_origin() const { return security_origin_; } - private: friend class base::RefCountedThreadSafe; @@ -88,7 +89,6 @@ class CONTENT_EXPORT Stream : public base::RefCountedThreadSafe { size_t data_bytes_read_; bool can_add_data_; - GURL security_origin_; GURL url_; scoped_refptr data_; diff --git a/content/browser/streams/stream_unittest.cc b/content/browser/streams/stream_unittest.cc index fda34d8..36add1d 100644 --- a/content/browser/streams/stream_unittest.cc +++ b/content/browser/streams/stream_unittest.cc @@ -93,7 +93,7 @@ TEST_F(StreamTest, SetReadObserver) { GURL url("blob://stream"); scoped_refptr stream( - new Stream(registry_.get(), &writer, GURL(), url)); + new Stream(registry_.get(), &writer, url)); EXPECT_TRUE(stream->SetReadObserver(&reader)); } @@ -104,7 +104,7 @@ TEST_F(StreamTest, SetReadObserver_SecondFails) { GURL url("blob://stream"); scoped_refptr stream( - new Stream(registry_.get(), &writer, GURL(), url)); + new Stream(registry_.get(), &writer, url)); EXPECT_TRUE(stream->SetReadObserver(&reader1)); EXPECT_FALSE(stream->SetReadObserver(&reader2)); } @@ -116,7 +116,7 @@ TEST_F(StreamTest, SetReadObserver_TwoReaders) { GURL url("blob://stream"); scoped_refptr stream( - new Stream(registry_.get(), &writer, GURL(), url)); + new Stream(registry_.get(), &writer, url)); EXPECT_TRUE(stream->SetReadObserver(&reader1)); // Once the first read observer is removed, a new one can be added. @@ -130,7 +130,7 @@ TEST_F(StreamTest, Stream) { GURL url("blob://stream"); scoped_refptr stream( - new Stream(registry_.get(), &writer, GURL(), url)); + new Stream(registry_.get(), &writer, url)); EXPECT_TRUE(stream->SetReadObserver(&reader)); const int kBufferSize = 1000000; @@ -150,7 +150,7 @@ TEST_F(StreamTest, GetStream) { GURL url("blob://stream"); scoped_refptr stream1( - new Stream(registry_.get(), &writer, GURL(), url)); + new Stream(registry_.get(), &writer, url)); scoped_refptr stream2 = registry_->GetStream(url); ASSERT_EQ(stream1, stream2); @@ -161,7 +161,7 @@ TEST_F(StreamTest, GetStream_Missing) { GURL url1("blob://stream"); scoped_refptr stream1( - new Stream(registry_.get(), &writer, GURL(), url1)); + new Stream(registry_.get(), &writer, url1)); GURL url2("blob://stream2"); scoped_refptr stream2 = registry_->GetStream(url2); @@ -173,7 +173,7 @@ TEST_F(StreamTest, CloneStream) { GURL url1("blob://stream"); scoped_refptr stream1( - new Stream(registry_.get(), &writer, GURL(), url1)); + new Stream(registry_.get(), &writer, url1)); GURL url2("blob://stream2"); ASSERT_TRUE(registry_->CloneStream(url2, url1)); @@ -186,7 +186,7 @@ TEST_F(StreamTest, CloneStream_Missing) { GURL url1("blob://stream"); scoped_refptr stream1( - new Stream(registry_.get(), &writer, GURL(), url1)); + new Stream(registry_.get(), &writer, url1)); GURL url2("blob://stream2"); GURL url3("blob://stream3"); @@ -200,7 +200,7 @@ TEST_F(StreamTest, UnregisterStream) { GURL url("blob://stream"); scoped_refptr stream1( - new Stream(registry_.get(), &writer, GURL(), url)); + new Stream(registry_.get(), &writer, url)); registry_->UnregisterStream(url); scoped_refptr stream2 = registry_->GetStream(url); diff --git a/content/browser/streams/stream_url_request_job_unittest.cc b/content/browser/streams/stream_url_request_job_unittest.cc index 5371735..4bb1798 100644 --- a/content/browser/streams/stream_url_request_job_unittest.cc +++ b/content/browser/streams/stream_url_request_job_unittest.cc @@ -99,7 +99,7 @@ class StreamURLRequestJobTest : public testing::Test { TEST_F(StreamURLRequestJobTest, TestGetSimpleDataRequest) { scoped_refptr stream( - new Stream(registry_.get(), NULL, GURL(), kStreamURL)); + new Stream(registry_.get(), NULL, kStreamURL)); scoped_refptr buffer( new net::StringIOBuffer(kTestData1)); @@ -112,7 +112,7 @@ TEST_F(StreamURLRequestJobTest, TestGetSimpleDataRequest) { TEST_F(StreamURLRequestJobTest, TestGetLargeStreamRequest) { scoped_refptr stream( - new Stream(registry_.get(), NULL, GURL(), kStreamURL)); + new Stream(registry_.get(), NULL, kStreamURL)); std::string large_data; large_data.reserve(kBufferSize * 5); @@ -141,7 +141,7 @@ TEST_F(StreamURLRequestJobTest, TestGetNonExistentStreamRequest) { TEST_F(StreamURLRequestJobTest, TestRangeDataRequest) { scoped_refptr stream( - new Stream(registry_.get(), NULL, GURL(), kStreamURL)); + new Stream(registry_.get(), NULL, kStreamURL)); scoped_refptr buffer( new net::StringIOBuffer(kTestData2)); @@ -157,7 +157,7 @@ TEST_F(StreamURLRequestJobTest, TestRangeDataRequest) { TEST_F(StreamURLRequestJobTest, TestInvalidRangeDataRequest) { scoped_refptr stream( - new Stream(registry_.get(), NULL, GURL(), kStreamURL)); + new Stream(registry_.get(), NULL, kStreamURL)); scoped_refptr buffer( new net::StringIOBuffer(kTestData2)); -- cgit v1.1