Use qmlProtectModule to protect a module from further modification

We don't need two mechanisms to do essentially the same thing.

QQmlTypeLoader::Blob::addImport() had an "optimization" to never check
for qmldir files of locked imports. This meant the first time you
imported a module with a plugin that locked the module you could use the
qmldir file to load additional .qml files afterwards. The second time
you imported the same thing, you couldn't. As this is not a great
example of consistent behavior, we drop this optimization and always
allow the qmldir files of plugins that lock the module to specify
additional QML files.

As a side effect of this, additional plugins listed in a qmldir file can
also now be loaded after the module has been locked by some other means.
However, any qmlRegisterFooBar() called from the module will be
prevented.

Change-Id: Idabb2bd5f75fc85b62f42625173672b4ae84382e
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

7 files changed, 32 insertions, 44 deletions

diff --git a/src/qml/qml/qqml.cpp b/src/qml/qml/qqml.cpp
index a3ca48d6d9..0f81d83f13 100644
--- a/src/qml/qml/qqml.cpp
+++ b/src/qml/qml/qqml.cpp
@@ -61,7 +61,7 @@ void qmlClearTypeRegistrations() // Declared in qqml.h
 //From qqml.h
 bool qmlProtectModule(const char *uri, int majVersion)
 {
-    return QQmlMetaType::protectModule(uri, majVersion);
+    return QQmlMetaType::protectModule(QString::fromUtf8(uri), majVersion);
 }
 
 //From qqml.h
diff --git a/src/qml/qml/qqmlimport.cpp b/src/qml/qml/qqmlimport.cpp
index 5feea9daa8..73c195cd13 100644
--- a/src/qml/qml/qqmlimport.cpp
+++ b/src/qml/qml/qqmlimport.cpp
@@ -1997,12 +1997,25 @@ void QQmlImportDatabase::setImportPathList(const QStringList &paths)
 /*!
     \internal
 */
-bool QQmlImportDatabase::registerPluginTypes(QObject *instance, const QString &basePath,
-                                      const QString &uri, const QString &typeNamespace, int vmaj, QList<QQmlError> *errors)
+static bool registerPluginTypes(QObject *instance, const QString &basePath, const QString &uri,
+                                const QString &typeNamespace, int vmaj, QList<QQmlError> *errors)
 {
     if (qmlImportTrace())
         qDebug().nospace() << "QQmlImportDatabase::registerPluginTypes: " << uri << " from " << basePath;
-    return QQmlMetaType::registerPluginTypes(instance, basePath, uri, typeNamespace, vmaj, errors);
+
+    if (!QQmlMetaType::registerPluginTypes(instance, basePath, uri, typeNamespace, vmaj, errors))
+        return false;
+
+    if (vmaj >= 0 && !typeNamespace.isEmpty() && !QQmlMetaType::protectModule(uri, vmaj)) {
+        QQmlError error;
+        error.setDescription(
+                    QString::fromLatin1("Cannot protect module %1 %2 as it was never registered")
+                    .arg(uri).arg(vmaj));
+        errors->append(error);
+        return false;
+    }
+
+    return true;
 }
 
 /*!
diff --git a/src/qml/qml/qqmlimport_p.h b/src/qml/qml/qqmlimport_p.h
index ea9c2eafb5..e24b3c447c 100644
--- a/src/qml/qml/qqmlimport_p.h
+++ b/src/qml/qml/qqmlimport_p.h
@@ -235,8 +235,6 @@ private:
                           const QString &baseName);
     bool importStaticPlugin(QObject *instance, const QString &basePath, const QString &uri,
                           const QString &typeNamespace, int vmaj, QList<QQmlError> *errors);
-    bool registerPluginTypes(QObject *instance, const QString &basePath,
-                          const QString &uri, const QString &typeNamespace, int vmaj, QList<QQmlError> *errors);
     void clearDirCache();
 
     struct QmldirCache {
diff --git a/src/qml/qml/qqmlmetatype.cpp b/src/qml/qml/qqmlmetatype.cpp
index 268d58a856..1a5affb0ad 100644
--- a/src/qml/qml/qqmlmetatype.cpp
+++ b/src/qml/qml/qqmlmetatype.cpp
@@ -342,7 +342,7 @@ QString registrationTypeString(QQmlType::RegistrationType typeType)
 
 // NOTE: caller must hold a QMutexLocker on "data"
 bool checkRegistration(QQmlType::RegistrationType typeType, QQmlMetaTypeData *data,
-                       const char *uri, const QString &typeName, int majorVersion = -1)
+                       const char *uri, const QString &typeName, int majorVersion)
 {
     if (!typeName.isEmpty()) {
         if (typeName.at(0).isLower()) {
@@ -363,27 +363,16 @@ bool checkRegistration(QQmlType::RegistrationType typeType, QQmlMetaTypeData *da
 
     if (uri && !typeName.isEmpty()) {
         QString nameSpace = QString::fromUtf8(uri);
-
-        if (data->typeRegistrationNamespace.isEmpty() && !nameSpace.isEmpty()) {
-            // Is the target namespace protected against further registrations?
-            if (data->protectedNamespaces.contains(nameSpace)) {
+        QQmlMetaTypeData::VersionedUri versionedUri;
+        versionedUri.uri = nameSpace;
+        versionedUri.majorVersion = majorVersion;
+        if (QQmlTypeModule* qqtm = data->uriToModule.value(versionedUri, 0)){
+            if (qqtm->isLocked()){
                 QString failure(QCoreApplication::translate("qmlRegisterType",
-                                                            "Cannot install %1 '%2' into protected namespace '%3'"));
-                data->recordTypeRegFailure(failure.arg(registrationTypeString(typeType)).arg(typeName).arg(nameSpace));
+                                                            "Cannot install %1 '%2' into protected module '%3' version '%4'"));
+                data->recordTypeRegFailure(failure.arg(registrationTypeString(typeType)).arg(typeName).arg(nameSpace).arg(majorVersion));
                 return false;
             }
-        } else if (majorVersion >= 0) {
-            QQmlMetaTypeData::VersionedUri versionedUri;
-            versionedUri.uri = nameSpace;
-            versionedUri.majorVersion = majorVersion;
-            if (QQmlTypeModule* qqtm = data->uriToModule.value(versionedUri, 0)){
-                if (qqtm->isLocked()){
-                    QString failure(QCoreApplication::translate("qmlRegisterType",
-                                                                "Cannot install %1 '%2' into protected module '%3' version '%4'"));
-                    data->recordTypeRegFailure(failure.arg(registrationTypeString(typeType)).arg(typeName).arg(nameSpace).arg(majorVersion));
-                    return false;
-                }
-            }
         }
     }
 
@@ -477,8 +466,10 @@ QQmlType QQmlMetaType::registerCompositeSingletonType(const QQmlPrivate::Registe
     bool fileImport = false;
     if (*(type.uri) == '\0')
         fileImport = true;
-    if (!checkRegistration(QQmlType::CompositeSingletonType, data, fileImport ? nullptr : type.uri, typeName))
+    if (!checkRegistration(QQmlType::CompositeSingletonType, data, fileImport ? nullptr : type.uri,
+                           typeName, type.versionMajor)) {
         return QQmlType();
+    }
 
     QQmlTypePrivate *priv = createQQmlType(data, typeName, type);
     addTypeToData(priv, data);
@@ -560,12 +551,12 @@ int QQmlMetaType::registerUnitCacheHook(
     return 0;
 }
 
-bool QQmlMetaType::protectModule(const char *uri, int majVersion)
+bool QQmlMetaType::protectModule(const QString &uri, int majVersion)
 {
     QQmlMetaTypeDataPtr data;
 
     QQmlMetaTypeData::VersionedUri versionedUri;
-    versionedUri.uri = QString::fromUtf8(uri);
+    versionedUri.uri = uri;
     versionedUri.majorVersion = majVersion;
 
     if (QQmlTypeModule* qqtm = data->uriToModule.value(versionedUri, 0)) {
@@ -683,8 +674,6 @@ bool QQmlMetaType::registerPluginTypes(QObject *instance, const QString &basePat
                 }
                 return false;
             }
-
-            data->protectedNamespaces.insert(uri);
         } else {
             // This is not an identified module - provide a warning
             qWarning().nospace() << qPrintable(
@@ -698,11 +687,9 @@ bool QQmlMetaType::registerPluginTypes(QObject *instance, const QString &basePat
                 = QQmlImports::urlFromLocalFileOrQrcOrUrl(basePath);
         }
 
-        data->typeRegistrationNamespace = typeNamespace;
         const QByteArray bytes = uri.toUtf8();
         const char *moduleId = bytes.constData();
         iface->registerTypes(moduleId);
-        data->typeRegistrationNamespace.clear();
     }
 
     if (!failures.isEmpty()) {
diff --git a/src/qml/qml/qqmlmetatype_p.h b/src/qml/qml/qqmlmetatype_p.h
index 6c2b0bb2a6..35d5386e1f 100644
--- a/src/qml/qml/qqmlmetatype_p.h
+++ b/src/qml/qml/qqmlmetatype_p.h
@@ -84,7 +84,7 @@ public:
     static void unregisterInternalCompositeType(QV4::ExecutableCompilationUnit *compilationUnit);
 
     static void registerModule(const char *uri, int versionMajor, int versionMinor);
-    static bool protectModule(const char *uri, int majVersion);
+    static bool protectModule(const QString &uri, int majVersion);
 
     static int typeId(const char *uri, int versionMajor, int versionMinor, const char *qmlName);
 
diff --git a/src/qml/qml/qqmlmetatypedata_p.h b/src/qml/qml/qqmlmetatypedata_p.h
index 5239b635ce..ea796ee7c6 100644
--- a/src/qml/qml/qqmlmetatypedata_p.h
+++ b/src/qml/qml/qqmlmetatypedata_p.h
@@ -106,10 +106,6 @@ struct QQmlMetaTypeData
     QList<QQmlPrivate::AutoParentFunction> parentFunctions;
     QVector<QQmlPrivate::QmlUnitCacheLookupFunction> lookupCachedQmlUnit;
 
-    QSet<QString> protectedNamespaces;
-
-    QString typeRegistrationNamespace;
-
     QHash<int, int> qmlLists;
 
     QHash<const QMetaObject *, QQmlPropertyCache *> propertyCaches;
diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp
index 42e7d2c4b4..5ab404f743 100644
--- a/src/qml/qml/qqmltypeloader.cpp
+++ b/src/qml/qml/qqmltypeloader.cpp
@@ -563,13 +563,7 @@ bool QQmlTypeLoader::Blob::addImport(QQmlTypeLoader::Blob::PendingImportPtr impo
         QString qmldirFilePath;
         QString qmldirUrl;
 
-        if (QQmlMetaType::isLockedModule(import->uri, import->majorVersion)) {
-            //Locked modules are checked first, to save on filesystem checks
-            if (!m_importCache.addLibraryImport(importDatabase, import->uri, import->qualifier, import->majorVersion,
-                                          import->minorVersion, QString(), QString(), false, errors))
-                return false;
-
-        } else if (m_importCache.locateQmldir(importDatabase, import->uri, import->majorVersion, import->minorVersion,
+        if (m_importCache.locateQmldir(importDatabase, import->uri, import->majorVersion, import->minorVersion,
                                  &qmldirFilePath, &qmldirUrl)) {
             // This is a local library import
             if (!m_importCache.addLibraryImport(importDatabase, import->uri, import->qualifier, import->majorVersion,