[packchk] Validate <url> starts with https:// #1109 (#1139) (#1961)

* [packchk] Validate <url> starts with https:// #1109

---------

Co-authored-by: Thorsten de Buhr <[email protected]>
Co-authored-by: Evgueni Driouk <[email protected]>

Author: 3 people

tools/packchk/include/CheckFiles.h

@@ -30,6 +30,7 @@ class CheckFiles {
   void SetPackagePath(const std::string& packagePath);
   const std::string& GetPackagePath() const;
   bool CheckFile(RteItem* item);
+  bool CheckUrls(RteItem* item);
   bool CheckFileExists(const std::string& fileName, int lineNo, bool associated = false);
   bool CheckCaseSense(const std::string& fileName, int lineNo);
   bool CheckFileIsInPack(const std::string& fileName, int lineNo);

tools/packchk/src/CheckFiles.cpp

@@ -40,6 +40,7 @@ CheckFilesVisitor::~CheckFilesVisitor()
 VISIT_RESULT CheckFilesVisitor::Visit(RteItem* item)
 {
   m_checkFiles.CheckFile(item);
+  m_checkFiles.CheckUrls(item);
 
   return VISIT_RESULT::CONTINUE_VISIT;
 }
@@ -170,6 +171,36 @@ bool CheckFiles::ToUpper(string& text)
   return true;
 }
 
+/**
+ * @brief check aspects of an RTE url item
+ * @param item RteItem item to check
+ * @return passed / failed
+*/
+bool CheckFiles::CheckUrls(RteItem* item)
+{
+  if(!item || item->GetTag() == "package") {
+    return true;
+  }
+
+  bool bOk = true;
+  const auto lineNo = item->GetLineNumber();
+  
+  if(item->GetText().find("http://", 0) != string::npos) {
+    LogMsg("M300", TAG(item->GetTag()), URL("https://"), lineNo);
+    bOk = false;
+  }
+    
+  const auto& attributes = item->GetAttributes();
+  for(const auto& [attr, text] : attributes) {
+    if(text.find("http://", 0) != string::npos) {
+      LogMsg("M300", TAG(attr), URL("https://"), lineNo);
+    bOk = false;
+    }
+  }
+
+  return bOk;
+}
+
 /**
  * @brief check aspects of an RTE file item
  * @param item RteItem item to check

tools/packchk/src/PackChk_Msgs.cpp

@@ -105,7 +105,7 @@ const MsgTable PackChk::msgTable = {
   { "M219", { MsgLevel::LEVEL_ERROR,    CRLF_B, "Unable to find specified schema file '%PATH%'"} },
 
 // 300... Validation Errors
-  { "M300", { MsgLevel::LEVEL_ERROR,    CRLF_B, "" } },
+  { "M300", { MsgLevel::LEVEL_WARNING,  CRLF_B, "%TAG% must use '%URL%'" } },
   { "M301", { MsgLevel::LEVEL_ERROR,    CRLF_B, "Checking Pack URL of PDSC file failed:\n"\
                                                 "  Expected URL : '%URL1%'\n"\
                                                 "  Package  URL : '%URL2%'" } },

tools/packchk/test/data/TestUrlHttps/TestVendor.TestUrlHttps.pdsc

@@ -0,0 +1,63 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<package schemaVersion="1.4" xmlns:xs="http://www.w3.org/2001/XMLSchema-instance" xs:noNamespaceSchemaLocation="PACK.xsd">
+  <vendor>TestVendor</vendor>
+  <url>http://www.testurl.foo/pack/</url>
+  <name>TestUrlHttps</name>
+  <description>TestUrlHttps</description>
+  <repository type="git">http://github.com/ARM-software/CMSIS-Driver.git</repository>
+
+  <releases>
+    <release version="2.1.0" date="2022-06-19" tag="2.1.0" url="http://github.com/ARM-software/CMSIS-Driver/archive/2.1.0.zip">
+      Added nothing.
+    </release>
+    <release version="0.0.1" date="2022-06-20">
+      Initial release of TestUrlHttps.
+    </release>
+  </releases>
+
+  <keywords>
+    <keyword>TestUrlHttps</keyword>
+  </keywords>
+
+  <conditions>
+    <condition id="Test_Condition">
+      <description>Test Device</description>
+      <require Dvendor="ARM:82"/>
+    </condition>
+  </conditions>
+
+  <components>
+    <component Cclass="TestClass" Cgroup="TestGlobal" Cversion="1.0.0" condition="Test_Condition">
+      <description>TestGlobal</description>
+    </component>
+    <component Cclass="Device" Cgroup="Config Tools" Csub="Init" Cversion="1.0.0" condition="Test_Condition" generator="MyConfig">
+      <description>Initialization generated by MyConfig</description>
+      <files>
+        <file category="doc" name="http://www.testurl.foo/web/myConfig"/>
+      </files>
+    </component>
+  </components>
+  
+  <devices>
+    <family Dfamily="MyDevice Series" Dvendor="MyDevice:99">
+      <processor Dcore="Cortex-M3" DcoreVersion="r1p1"/>
+      <book category="overview"  name="http://www.testurl.foo/web/myDevice" title="MyDevice Web Page"/>
+
+      <subFamily DsubFamily="MyDevice">
+        <device Dname="MyDeviceM0">
+          <processor Dfpu="0" Dmpu="0" Dendian="Little-endian" Dclock="24000000"/>
+        </device>
+      </subFamily>
+    </family>
+  </devices>
+
+  <boards>
+    <board vendor="ARM" name="MyBoard" orderForm="http://www.testurl.foo">
+      <description>MyBoard Evaluation Kit</description>
+      <image small="http://www.testurl.foo/web/myBoardSmall.png"/>
+      <book category="overview" name="http://www.testurl.foo/web/myBoard" title="MyBoard" public="true"/>
+    </board>
+  </boards>
+
+</package>

tools/packchk/test/integtests/src/PackChkIntegTests.cpp

@@ -764,6 +764,36 @@ TEST_F(PackChkIntegTests, CheckDuplicateFlashAlgo) {
   }
 }
 
+// Validate URLs to start with 'https://'
+TEST_F(PackChkIntegTests, CheckUrlForHttp) {
+  const char* argv[5];
+
+  const string& pdscFile = PackChkIntegTestEnv::localtestdata_dir +
+    "/TestUrlHttps/TestVendor.TestUrlHttps.pdsc";
+  ASSERT_TRUE(RteFsUtils::Exists(pdscFile));
+
+  argv[0] = (char*)"";
+  argv[1] = (char*)pdscFile.c_str();
+  argv[2] = (char*)"--disable-validation";
+  argv[3] = (char*)"-x";
+  argv[4] = (char*)"!M368";
+
+  PackChk packChk;
+  EXPECT_EQ(1, packChk.Check(2, argv, nullptr));
+
+  auto errMsgs = ErrLog::Get()->GetLogMessages();
+  int M300_foundCnt = 0;
+  for (const string& msg : errMsgs) {
+    size_t s;
+    if ((s = msg.find("M300")) != string::npos) {
+      M300_foundCnt++;
+    }
+  }
+
+  if (M300_foundCnt != 8) {
+    FAIL() << "error: missing message M300";
+  }
+}
 
 // Validate file is .md
 TEST_F(PackChkIntegTests, CheckDescriptionOverviewMd) {
@@ -793,7 +823,6 @@ TEST_F(PackChkIntegTests, CheckDescriptionOverviewMd) {
     FAIL() << "error: message M337 found";
   }
 }
-
 // Validate config file vs. include path
 TEST_F(PackChkIntegTests, CheckConfigFileInIncludePath) {
   const char* argv[3];