Let package_ucp::ContentProvider::createPackage throw exceptions on failure

...instead of returning a null XHierarchicalNameAccess.  Otherwise, UCB's
globalTransfer from a vnd.sun.star.package URL that denotes a file that is not a
zip file to a file URL folder (i.e., to extract the zip content) silently
succeeds but just creates an empty file in the target folder.  That, in turn,
causes "unopkg add foo.oxt", where foo.oxt is a file that is not a zip file, to
silently succeed and add an "empty" extension.

This change is somewhat bold in that it changes createPackage from "can return
empty reference" to "never returns empty reference, but can throw
RuntimeException."  Especially, it considers "empty name" as a (silent)
violation of its contract with its caller now.  I hope this does not affect any
legitimate scenarios---at least, it does not break a "make check" here.  (In
turn, the two package_ucp::Content::getPackage overloads change to never return
a null reference, either.  And I changed the parameters of createPackage, seeing
that all call-sites pass in some PackageUri's getPackage()/getParam() pair.)

Change-Id: I0eab5f8059dfedefc7030da38da528ba21ea8d79

ucb/source/ucp/package/pkgcontent.cxx

@@ -217,7 +217,7 @@ Content* Content::create(
 
     uno::Reference< container::XHierarchicalNameAccess > xPackage;
 
-    xPackage = pProvider->createPackage( aURI.getPackage(), aURI.getParam() );
+    xPackage = pProvider->createPackage( aURI );
 
     uno::Reference< ucb::XContentIdentifier > xId
         = new ::ucbhelper::ContentIdentifier( aURI.getUri() );
@@ -2217,12 +2217,12 @@ uno::Reference< container::XHierarchicalNameAccess > Content::getPackage(
     if ( rURI.getPackage() == m_aUri.getPackage() )
     {
         if ( !m_xPackage.is() )
-            m_xPackage = m_pProvider->createPackage( m_aUri.getPackage(), m_aUri.getParam() );
+            m_xPackage = m_pProvider->createPackage( m_aUri );
 
         return m_xPackage;
     }
 
-    return m_pProvider->createPackage( rURI.getPackage(), rURI.getParam() );
+    return m_pProvider->createPackage( rURI );
 }
 
 //=========================================================================
@@ -2238,10 +2238,7 @@ sal_Bool Content::hasData(
             const PackageUri& rURI,
             uno::Reference< container::XHierarchicalNameAccess > & rxPackage )
 {
-    rxPackage = pProvider->createPackage( rURI.getPackage(), rURI.getParam() );
-    if ( !rxPackage.is() )
-        return sal_False;
-
+    rxPackage = pProvider->createPackage( rURI );
     return rxPackage->hasByHierarchicalName( rURI.getPath() );
 }
 
@@ -2254,9 +2251,6 @@ sal_Bool Content::hasData( const PackageUri& rURI )
     if ( rURI.getPackage() == m_aUri.getPackage() )
     {
         xPackage = getPackage();
-        if ( !xPackage.is() )
-            return sal_False;
-
         return xPackage->hasByHierarchicalName( rURI.getPath() );
     }
 
@@ -2271,9 +2265,7 @@ sal_Bool Content::loadData(
             ContentProperties& rProps,
             uno::Reference< container::XHierarchicalNameAccess > & rxPackage )
 {
-    rxPackage = pProvider->createPackage( rURI.getPackage(), rURI.getParam() );
-    if ( !rxPackage.is() )
-        return sal_False;
+    rxPackage = pProvider->createPackage( rURI );
 
     if ( rURI.isRootFolder() )
     {
@@ -2469,8 +2461,6 @@ sal_Bool Content::renameData(
     PackageUri aURI( xOldId->getContentIdentifier() );
     uno::Reference< container::XHierarchicalNameAccess > xNA = getPackage(
                                                                         aURI );
-    if ( !xNA.is() )
-        return sal_False;
 
     if ( !xNA->hasByHierarchicalName( aURI.getPath() ) )
         return sal_False;
@@ -2508,8 +2498,6 @@ sal_Bool Content::storeData( const uno::Reference< io::XInputStream >& xStream )
     osl::Guard< osl::Mutex > aGuard( m_aMutex );
 
     uno::Reference< container::XHierarchicalNameAccess > xNA = getPackage();
-    if ( !xNA.is() )
-        return sal_False;
 
     uno::Reference< beans::XPropertySet > xPackagePropSet(
                                                     xNA, uno::UNO_QUERY );
@@ -2740,8 +2728,6 @@ sal_Bool Content::removeData()
     osl::Guard< osl::Mutex > aGuard( m_aMutex );
 
     uno::Reference< container::XHierarchicalNameAccess > xNA = getPackage();
-    if ( !xNA.is() )
-        return sal_False;
 
     PackageUri aParentUri( getParentURL() );
     if ( !xNA->hasByHierarchicalName( aParentUri.getPath() ) )
@@ -2785,8 +2771,6 @@ sal_Bool Content::flushData()
     //       by the single entries. Maybe this has to change...
 
     uno::Reference< container::XHierarchicalNameAccess > xNA = getPackage();
-    if ( !xNA.is() )
-        return sal_False;
 
     uno::Reference< util::XChangesBatch > xBatch( xNA, uno::UNO_QUERY );
     if ( !xBatch.is() )
@@ -2815,8 +2799,6 @@ uno::Reference< io::XInputStream > Content::getInputStream()
 
     uno::Reference< io::XInputStream > xStream;
     uno::Reference< container::XHierarchicalNameAccess > xNA = getPackage();
-    if ( !xNA.is() )
-        return xStream;
 
     if ( !xNA->hasByHierarchicalName( m_aUri.getPath() ) )
         return xStream;
@@ -2854,8 +2836,6 @@ uno::Reference< container::XEnumeration > Content::getIterator()
 
     uno::Reference< container::XEnumeration > xIter;
     uno::Reference< container::XHierarchicalNameAccess > xNA = getPackage();
-    if ( !xNA.is() )
-        return xIter;
 
     if ( !xNA->hasByHierarchicalName( m_aUri.getPath() ) )
         return xIter;

ucb/source/ucp/package/pkgprovider.cxx

@@ -30,6 +30,7 @@
 #include <cppuhelper/weak.hxx>
 #include <ucbhelper/contentidentifier.hxx>
 #include <com/sun/star/container/XHierarchicalNameAccess.hpp>
+#include <com/sun/star/lang/WrappedTargetRuntimeException.hpp>
 #include "pkgprovider.hxx"
 #include "pkgcontent.hxx"
 #include "pkguri.hxx"
@@ -232,17 +233,11 @@ uno::Reference< ucb::XContent > SAL_CALL ContentProvider::queryContent(
 //=========================================================================
 
 uno::Reference< container::XHierarchicalNameAccess >
-ContentProvider::createPackage( const OUString & rName, const OUString & rParam )
+ContentProvider::createPackage( const PackageUri & rURI )
 {
     osl::MutexGuard aGuard( m_aMutex );
 
-    if ( rName.isEmpty() )
-    {
-        OSL_FAIL( "ContentProvider::createPackage - Invalid URL!" );
-        return uno::Reference< container::XHierarchicalNameAccess >();
-    }
-
-    OUString rURL = rName + rParam;
+    OUString rURL = rURI.getPackage() + rURI.getParam();
 
     if ( m_pPackages )
     {
@@ -257,43 +252,30 @@ ContentProvider::createPackage( const OUString & rName, const OUString & rParam
         m_pPackages = new Packages;
 
     // Create new package...
+    uno::Sequence< uno::Any > aArguments( 1 );
+    aArguments[ 0 ] <<= rURL;
+    uno::Reference< container::XHierarchicalNameAccess > xNameAccess;
     try
     {
-        uno::Sequence< uno::Any > aArguments( 1 );
-        aArguments[ 0 ] <<= rURL;
-
-        uno::Reference< uno::XInterface > xIfc
-            = m_xContext->getServiceManager()->createInstanceWithArgumentsAndContext("com.sun.star.packages.comp.ZipPackage",
-                aArguments, m_xContext );
-
-        if ( xIfc.is() )
-        {
-            uno::Reference<
-                container::XHierarchicalNameAccess > xNameAccess(
-                                                        xIfc, uno::UNO_QUERY );
-
-            OSL_ENSURE( xNameAccess.is(),
-                        "ContentProvider::createPackage - "
-                        "Got no hierarchical name access!" );
-
-            rtl::Reference< Package> xPackage
-                = new Package( rURL, xNameAccess, this );
-
-            (*m_pPackages)[ rURL ] = xPackage.get();
-
-            return xPackage.get();
-        }
+        xNameAccess = uno::Reference< container::XHierarchicalNameAccess >(
+            m_xContext->getServiceManager()->createInstanceWithArgumentsAndContext(
+                "com.sun.star.packages.comp.ZipPackage",
+                aArguments, m_xContext ),
+            css::uno::UNO_QUERY_THROW );
     }
     catch ( uno::RuntimeException const & )
     {
-        // createInstanceWithArguemts
+        throw;
     }
-    catch ( uno::Exception const & )
+    catch ( uno::Exception const & e )
     {
-        // createInstanceWithArguemts
+        throw css::lang::WrappedTargetRuntimeException(
+            e.Message, e.Context, css::uno::makeAny(e));
     }
 
-    return uno::Reference< container::XHierarchicalNameAccess >();
+    rtl::Reference< Package> xPackage = new Package( rURL, xNameAccess, this );
+    (*m_pPackages)[ rURL ] = xPackage.get();
+    return xPackage.get();
 }
 
 //=========================================================================

ucb/source/ucp/package/pkgprovider.hxx

@@ -87,7 +87,7 @@ public:
 
     ::com::sun::star::uno::Reference<
         ::com::sun::star::container::XHierarchicalNameAccess >
-    createPackage( const OUString & rName, const OUString & rParam );
+    createPackage( const PackageUri & rParam );
     sal_Bool
     removePackage( const OUString & rName );
 };