Created a blocking copy to temporary file function.
This functionality will be useful for creating temporary nexe files
from the URLResponse in the nexe content handler.
This change also modifies the SFI and nonsfi content handlers to use this
new function.
BUG=https://github.com/domokit/mojo/issues/396
R=mseaborn@chromium.org
Review URL: https://codereview.chromium.org/1303343007 .
diff --git a/mojo/data_pipe_utils/data_pipe_file_utils.cc b/mojo/data_pipe_utils/data_pipe_file_utils.cc
index e37c8df..08b24a3 100644
--- a/mojo/data_pipe_utils/data_pipe_file_utils.cc
+++ b/mojo/data_pipe_utils/data_pipe_file_utils.cc
@@ -315,18 +315,24 @@
} // namespace
-bool BlockingCopyToFile(ScopedDataPipeConsumerHandle source,
- const base::FilePath& destination) {
- TRACE_EVENT1("data_pipe_utils", "BlockingCopyToFile", "dest",
- destination.MaybeAsASCII());
- base::ScopedFILE fp(base::OpenFile(destination, "wb"));
+base::ScopedFILE BlockingCopyToTempFile(ScopedDataPipeConsumerHandle source) {
+ base::FilePath path;
+ base::ScopedFILE fp(CreateAndOpenTemporaryFile(&path));
if (!fp) {
- LOG(ERROR) << "OpenFile('" << destination.value()
- << "'failed in BlockingCopyToFile";
- return false;
+ LOG(ERROR) << "CreateAndOpenTemporaryFile failed in"
+ << "BlockingCopyToTempFile";
+ return nullptr;
}
- return BlockingCopyHelper(source.Pass(),
- base::Bind(&CopyToFileHelper, fp.get()));
+ if (unlink(path.value().c_str())) {
+ LOG(ERROR) << "Failed to unlink temporary file";
+ return nullptr;
+ }
+ if (!BlockingCopyHelper(source.Pass(),
+ base::Bind(&CopyToFileHelper, fp.get()))) {
+ LOG(ERROR) << "Could not copy source to temporary file";
+ return nullptr;
+ }
+ return fp;
}
void CopyToFile(ScopedDataPipeConsumerHandle source,
diff --git a/mojo/data_pipe_utils/data_pipe_utils.h b/mojo/data_pipe_utils/data_pipe_utils.h
index 289fe12..48c59d9 100644
--- a/mojo/data_pipe_utils/data_pipe_utils.h
+++ b/mojo/data_pipe_utils/data_pipe_utils.h
@@ -8,6 +8,7 @@
#include <string>
#include "base/callback_forward.h"
+#include "base/files/scoped_file.h"
#include "base/threading/platform_thread.h"
#include "mojo/public/cpp/system/core.h"
@@ -42,11 +43,11 @@
bool BlockingCopyFromString(const std::string& source,
const ScopedDataPipeProducerHandle& destination);
-// Synchronously copies data from source to the destination file returning true
-// on success and false on error. In case of an error, |destination| holds the
-// data that could be read from the source before the error occured.
-bool BlockingCopyToFile(ScopedDataPipeConsumerHandle source,
- const base::FilePath& destination);
+// Synchronously copies source data to a temporary file, returning a file
+// pointer on success and NULL on error. The temporary file is unlinked
+// immediately so that it is only accessible by file pointer (and removed once
+// closed or the creating process dies).
+base::ScopedFILE BlockingCopyToTempFile(ScopedDataPipeConsumerHandle source);
} // namespace common
} // namespace mojo
diff --git a/services/nacl/content_handler_main.cc b/services/nacl/content_handler_main.cc
index 776ebd8..b364422 100644
--- a/services/nacl/content_handler_main.cc
+++ b/services/nacl/content_handler_main.cc
@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "base/files/file_path.h"
#include "base/files/file_util.h"
+#include "base/files/scoped_file.h"
#include "base/message_loop/message_loop.h"
#include "build/build_config.h"
#include "mojo/application/application_runner_chromium.h"
@@ -24,24 +24,8 @@
namespace content_handler {
namespace {
-bool URLResponseToTempFile(mojo::URLResponsePtr& response,
- base::FilePath* file_path) {
- if (!base::CreateTemporaryFile(file_path)) {
- return false;
- }
-
- if (!mojo::common::BlockingCopyToFile(response->body.Pass(), *file_path)) {
- base::DeleteFile(*file_path, false);
- return false;
- }
-
- // TODO(ncbray): can we ensure temp file deletion even if we crash?
- return true;
-}
-
-bool TempFileForURL(mojo::URLLoaderPtr& url_loader,
- const GURL& url,
- base::FilePath* path) {
+base::ScopedFILE TempFileForURL(mojo::URLLoaderPtr& url_loader,
+ const GURL& url) {
mojo::URLRequestPtr request(mojo::URLRequest::New());
request->url = url.spec();
request->method = "GET";
@@ -61,22 +45,35 @@
LOG(ERROR) << "could not load " << url.spec() << " ("
<< response->error->code << ") "
<< response->error->description.get().c_str();
- return false;
+ return nullptr;
}
- return URLResponseToTempFile(response, path);
+ return mojo::common::BlockingCopyToTempFile(response->body.Pass());
}
-NaClDesc* NaClDescFromNexePath(base::FilePath& path) {
- base::File file(path, base::File::FLAG_OPEN | base::File::FLAG_READ |
- base::File::FLAG_EXECUTE);
- if (!file.IsValid()) {
- LOG(FATAL) << "failed to open " << path.value();
+NaClDesc* FileStreamToNaClDesc(FILE* file_stream) {
+ // Get file handle
+ int fd = fileno(file_stream);
+ if (fd == -1) {
+ LOG(FATAL) << "Could not open the stream pointer's file descriptor";
}
- // Note: potentially unsafe, assumes the file is immutable. This should be
- // OK because we're currently only using temp files.
- return NaClDescCreateWithFilePathMetadata(file.TakePlatformFile(),
- path.AsUTF8Unsafe().c_str());
+
+ // These file descriptors must be dup'd, since NaClDesc takes ownership
+ // of the file descriptor. The descriptor is still needed to close
+ // the file stream.
+ fd = dup(fd);
+ if (fd == -1) {
+ LOG(FATAL) << "Could not dup the file descriptor";
+ }
+
+ NaClDesc* desc = NaClDescCreateWithFilePathMetadata(fd, "");
+
+ // Clean up.
+ if (fclose(file_stream)) {
+ LOG(FATAL) << "Failed to close temp file";
+ }
+
+ return desc;
}
} // namespace
@@ -109,8 +106,9 @@
LOG(FATAL) << "Cannot resolve IRT URL";
}
- if (!TempFileForURL(url_loader, irt_url, &irt_path_)) {
- LOG(FATAL) << "Could not aquire the IRT";
+ irt_fp_ = TempFileForURL(url_loader, irt_url);
+ if (!irt_fp_) {
+ LOG(FATAL) << "Could not acquire the IRT";
}
}
@@ -128,35 +126,25 @@
// Needed to use Mojo interfaces on this thread.
base::MessageLoop loop(mojo::common::MessagePumpMojo::Create());
- // Aquire the nexe.
- base::FilePath nexe_path;
- if (!URLResponseToTempFile(response, &nexe_path)) {
+ // Acquire the nexe.
+ base::ScopedFILE nexe_fp =
+ mojo::common::BlockingCopyToTempFile(response->body.Pass());
+ if (!nexe_fp) {
LOG(FATAL) << "could not redirect nexe to temp file";
}
- // Aquire the IRT.
+ // Acquire the IRT.
mojo::URLLoaderPtr url_loader = url_loader_.Pass();
LoadIRT(url_loader);
- // Get file handles to the IRT and nexe.
- NaClDesc* irt_desc = NaClDescFromNexePath(irt_path_);
- NaClDesc* nexe_desc = NaClDescFromNexePath(nexe_path);
+ NaClDesc* irt_desc = FileStreamToNaClDesc(irt_fp_.release());
+ NaClDesc* nexe_desc = FileStreamToNaClDesc(nexe_fp.release());
// Run.
int exit_code = mojo::LaunchNaCl(
- nexe_desc, irt_desc, 0, NULL,
+ nexe_desc, irt_desc, 0, nullptr,
application_request.PassMessagePipe().release().value());
- // TODO(ncbray): are the file handles actually closed at this point?
-
- // Clean up.
- if (!base::DeleteFile(irt_path_, false)) {
- LOG(FATAL) << "Failed to remove irt temp file " << irt_path_.value();
- }
- if (!base::DeleteFile(nexe_path, false)) {
- LOG(FATAL) << "Failed to remove nexe temp file " << nexe_path.value();
- }
-
// Exits the process cleanly, does not return.
mojo::NaClExit(exit_code);
NOTREACHED();
@@ -164,7 +152,7 @@
mojo::ContentHandlerFactory content_handler_factory_;
GURL url_;
- base::FilePath irt_path_;
+ base::ScopedFILE irt_fp_;
mojo::URLLoaderPtr url_loader_;
diff --git a/services/nacl/content_handler_main_nonsfi.cc b/services/nacl/content_handler_main_nonsfi.cc
index 5039369..62a907d 100644
--- a/services/nacl/content_handler_main_nonsfi.cc
+++ b/services/nacl/content_handler_main_nonsfi.cc
@@ -17,35 +17,6 @@
namespace nacl {
namespace content_handler {
-namespace {
-
-// Copies response (input) into new temporary file open by fd (output).
-bool URLResponseToTempFile(mojo::URLResponsePtr& response, int* fd) {
- base::FilePath path;
- if (!base::CreateTemporaryFile(&path)) {
- return false;
- }
-
- if (!mojo::common::BlockingCopyToFile(response->body.Pass(), path)) {
- base::DeleteFile(path, false);
- return false;
- }
- *fd = open(path.value().c_str(), O_RDONLY);
- if (*fd < 0) {
- LOG(FATAL) << "Failed to open " << path.value().c_str() << ": "
- << strerror(errno) << "\n";
- }
-
- // Since we unlink an open file, the temp file will be removed as soon as the
- // fd is closed.
- if (unlink(path.value().c_str())) {
- LOG(FATAL) << "Failed to unlink " << path.value().c_str() << ": "
- << strerror(errno) << "\n";
- }
- return true;
-}
-
-} // namespace
class NaClContentHandler : public mojo::ApplicationDelegate,
public mojo::ContentHandlerFactory::Delegate {
@@ -70,9 +41,22 @@
// Needed to use Mojo interfaces on this thread.
base::MessageLoop loop(mojo::common::MessagePumpMojo::Create());
// Acquire the nexe.
- int fd;
- if (!URLResponseToTempFile(response, &fd)) {
- LOG(FATAL) << "could not redirect nexe to temp file";
+ base::ScopedFILE nexe_fp =
+ mojo::common::BlockingCopyToTempFile(response->body.Pass());
+ if (!nexe_fp) {
+ LOG(FATAL) << "Could not redirect nexe to temp file";
+ }
+ FILE* nexe_file_stream = nexe_fp.release();
+ int fd = fileno(nexe_file_stream);
+ if (fd == -1) {
+ LOG(FATAL) << "Could not open the stream pointer's file descriptor";
+ }
+ fd = dup(fd);
+ if (fd == -1) {
+ LOG(FATAL) << "Could not dup the file descriptor";
+ }
+ if (fclose(nexe_file_stream)) {
+ LOG(FATAL) << "Failed to close temp file";
}
// Run -- also, closes the fd, removing the temp file.
