net: Fix a regression that broke file uploading with http authentication

This is a reland of r138168, which was reverted speculatively, but identified unrelated.

When uploading a file with http authentication, UploadData is reused
for a new UploadDataStream. Hence, we should close the files in UploadData
if already opened and read, so we can reread the files from the beginning.

The regression was caused by r123677.  Special thanks to thomas.themel for
identifying the offending patch and helping us to fix the issue.

BUG=128574
TEST=added a unit test; manually confirmed that uploading a file with basic authentication, and multi-round authentication with NTLM and Negotiate worked.

Review URL: https://chromiumcodereview.appspot.com/10408042

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@138191 0039d316-1c4b-4281-b951-d872f2087c98

net/base/upload_data.cc

@@ -114,6 +114,20 @@ uint64 UploadData::Element::BytesRemaining() {
   return GetContentLength() - offset_;
 }
 
+void UploadData::Element::ResetOffset() {
+  offset_ = 0;
+
+  // Delete the file stream if already opened, so we can reread the file from
+  // the beginning.
+  if (file_stream_) {
+    // Temporarily allow until fix: http://crbug.com/72001.
+    base::ThreadRestrictions::ScopedAllowIO allow_io;
+    file_stream_->CloseSync();
+    delete file_stream_;
+    file_stream_ = NULL;
+  }
+}
+
 FileStream* UploadData::Element::OpenFileStream() {
   scoped_ptr<FileStream> file(new FileStream(NULL));
   int64 rv = file->OpenSync(

net/base/upload_data.h

@@ -129,8 +129,9 @@ class NET_EXPORT UploadData
     // Returns the number of bytes remaining to read.
     uint64 BytesRemaining();
 
-    // Resets the offset to zero, so that the element can be reread.
-    void ResetOffset() { offset_ = 0; }
+    // Resets the offset to zero and closes the file stream if opened, so
+    // that the element can be reread.
+    void ResetOffset();
 
    private:
     // Returns a FileStream opened for reading for this element, positioned

net/base/upload_data_stream_unittest.cc

@@ -27,6 +27,17 @@ const char kTestData[] = "0123456789";
 const size_t kTestDataSize = arraysize(kTestData) - 1;
 const size_t kTestBufferSize = 1 << 14;  // 16KB.
 
+// Reads data from the upload data stream, and returns the data as string.
+std::string ReadFromUploadDataStream(UploadDataStream* stream) {
+  std::string data_read;
+  scoped_refptr<IOBuffer> buf = new IOBuffer(kTestBufferSize);
+  while (!stream->IsEOF()) {
+    const int bytes_read = stream->Read(buf, kTestBufferSize);
+    data_read.append(buf->data(), bytes_read);
+  }
+  return data_read;
+}
+
 }  // namespace
 
 class UploadDataStreamTest : public PlatformTest {
@@ -144,4 +155,36 @@ TEST_F(UploadDataStreamTest, FileChanged) {
   file_util::Delete(temp_file_path, false);
 }
 
+TEST_F(UploadDataStreamTest, UploadDataReused) {
+  FilePath temp_file_path;
+  ASSERT_TRUE(file_util::CreateTemporaryFile(&temp_file_path));
+  ASSERT_EQ(static_cast<int>(kTestDataSize),
+            file_util::WriteFile(temp_file_path, kTestData, kTestDataSize));
+
+  // Prepare |upload_data_| that contains a file.
+  std::vector<UploadData::Element> elements;
+  UploadData::Element element;
+  element.SetToFilePath(temp_file_path);
+  elements.push_back(element);
+  upload_data_->SetElements(elements);
+  EXPECT_EQ(kTestDataSize, upload_data_->GetContentLengthSync());
+
+  // Confirm that the file is read properly.
+  {
+    UploadDataStream stream(upload_data_);
+    ASSERT_EQ(OK, stream.Init());
+    EXPECT_EQ(kTestData, ReadFromUploadDataStream(&stream));
+  }
+
+  // Reuse |upload_data_| for another UploadDataStream, and confirm that the
+  // file is read properly.
+  {
+    UploadDataStream stream(upload_data_);
+    ASSERT_EQ(OK, stream.Init());
+    EXPECT_EQ(kTestData, ReadFromUploadDataStream(&stream));
+  }
+
+  file_util::Delete(temp_file_path, false);
+}
+
 }  // namespace net