[master] Merge branch 'trac2572-fix'

src/lib/dns/master_lexer.cc

@@ -144,7 +144,13 @@ MasterLexer::pushSource(const char* filename, std::string* error) {
 
 void
 MasterLexer::pushSource(std::istream& input) {
-    impl_->sources_.push_back(InputSourcePtr(new InputSource(input)));
+    try {
+        impl_->sources_.push_back(InputSourcePtr(new InputSource(input)));
+    } catch (const InputSource::OpenError& ex) {
+        // Convert the "internal" exception to public one.
+        isc_throw(Unexpected, "Failed to push a stream to lexer: " <<
+                  ex.what());
+    }
     impl_->source_ = impl_->sources_.back().get();
     impl_->has_previous_ = false;
     impl_->last_was_eol_ = true;

src/lib/dns/master_lexer.h

@@ -401,6 +401,22 @@ public:
     /// The behavior of the lexer is undefined if the caller builds or adds
     /// data in \c input after pushing it.
     ///
+    /// Except for rare case system errors such as memory allocation failure,
+    /// this method is generally expected to be exception free.  However,
+    /// it can still throw if it encounters an unexpected failure when it
+    /// tries to identify the "size" of the input source (see
+    /// \c getTotalSourceSize()).  It's an unexpected result unless the
+    /// caller intentionally passes a broken stream; otherwise it would mean
+    /// some system-dependent unexpected behavior or possibly an internal bug.
+    /// In these cases it throws an \c Unexpected exception.  Note that
+    /// this version of the method doesn't return a boolean unlike the
+    /// other version that takes a file name; since this failure is really
+    /// unexpected and can be critical, it doesn't make sense to give the
+    /// caller an option to continue (other than by explicitly catching the
+    /// exception).
+    ///
+    /// \throw Unexpected An unexpected failure happens in initialization.
+    ///
     /// \param input An input stream object that produces textual
     /// representation of DNS RRs.
     void pushSource(std::istream& input);

src/lib/dns/master_lexer_inputsource.cc

@@ -36,33 +36,50 @@ createStreamName(const std::istream& input_stream) {
 
 size_t
 getStreamSize(std::istream& is) {
+    errno = 0;                  // see below
     is.seekg(0, std::ios_base::end);
     if (is.bad()) {
         // This means the istream has an integrity error.  It doesn't make
         // sense to continue from this point, so we treat it as a fatal error.
         isc_throw(InputSource::OpenError,
                   "failed to seek end of input source");
-    } else if (is.fail()) {
+    } else if (is.fail() || errno != 0) {
         // This is an error specific to seekg().  There can be several
         // reasons, but the most likely cause in this context is that the
         // stream is associated with a special type of file such as a pipe.
         // In this case, it's more likely that other main operations of
         // the input source work fine, so we continue with just setting
         // the stream size to "unknown".
+        //
+        // (At least some versions of) Solaris + SunStudio shows deviant
+        // behavior here: seekg() apparently calls lseek(2) internally, but
+        // even if it fails it doesn't set the error bits of istream. That will
+        // confuse the rest of this function, so, as a heuristic workaround
+        // we check errno and handle any non 0 value as fail().
         is.clear();   // clear this error not to confuse later ops.
         return (MasterLexer::SOURCE_SIZE_UNKNOWN);
     }
     const std::streampos len = is.tellg();
+    size_t ret = len;
     if (len == static_cast<std::streampos>(-1)) { // cast for some compilers
-        isc_throw(InputSource::OpenError, "failed to get input size");
+        if (!is.fail()) {
+            // tellg() returns -1 if istream::fail() would be true, but it's
+            // not guaranteed that it shouldn't be returned in other cases.
+            // In fact, with the combination of SunStudio and stlport,
+            // a stringstream created by the default constructor showed that
+            // behavior.  We treat such cases as an unknown size.
+            ret = MasterLexer::SOURCE_SIZE_UNKNOWN;
+        } else {
+            isc_throw(InputSource::OpenError, "failed to get input size");
+        }
     }
     is.seekg(0, std::ios::beg);
     if (is.fail()) {
         isc_throw(InputSource::OpenError,
                   "failed to seek beginning of input source");
     }
-    assert(len >= 0);
-    return (len);
+    assert(len >= 0 || ret == MasterLexer::SOURCE_SIZE_UNKNOWN);
+    return (ret);
 }
 
 } // end of unnamed namespace

src/lib/dns/tests/master_lexer_unittest.cc

@@ -80,6 +80,15 @@ TEST_F(MasterLexerTest, pushStream) {
     checkEmptySource(lexer);
 }
 
+TEST_F(MasterLexerTest, pushStreamFail) {
+    // Pretend a "bad" thing happened in the stream.  This will make the
+    // initialization throw an exception.
+    ss << "test";
+    ss.setstate(std::ios_base::badbit);
+
+    EXPECT_THROW(lexer.pushSource(ss), isc::Unexpected);
+}
+
 TEST_F(MasterLexerTest, pushFile) {
     // We use zone file (-like) data, but in this test that actually doesn't
     // matter.