From a025d38ea54eea1b506918f5f5777600c9a4de92 Mon Sep 17 00:00:00 2001 From: Joao Afonso <joao.afonso@cern.ch> Date: Fri, 2 Dec 2022 17:08:45 +0100 Subject: [PATCH] Modify CTA code to enforce VID uppercase --- ReleaseNotes.md | 2 + catalogue/CatalogueTest.cpp | 20 +++---- catalogue/RdbmsCatalogue.cpp | 4 ++ common/utils/utils.cpp | 9 ++++ common/utils/utils.hpp | 8 +++ scheduler/GenericSchedulerTest.cpp | 14 ++--- scheduler/SchedulerTest.cpp | 52 +++++++++---------- .../daemon/DataTransferSessionTest.cpp | 8 +-- tapeserver/tapelabel/TapeLabelCmdLineArgs.cpp | 28 ++++++---- 9 files changed, 87 insertions(+), 58 deletions(-) diff --git a/ReleaseNotes.md b/ReleaseNotes.md index ac6e07102f..39c03a6f93 100644 --- a/ReleaseNotes.md +++ b/ReleaseNotes.md @@ -1,6 +1,8 @@ # v4.NEXT ## Summary +### Features +- cta/CTA#230 - Modify CTA code to enforce VID uppercase ### Continuous Integration - cta/CTA#205 - Updating EOS4/EOS4 in versionlock for v4.8.95/v5.1.4 ### Catalogue Schema diff --git a/catalogue/CatalogueTest.cpp b/catalogue/CatalogueTest.cpp index c1ee3ac8f4..2a2ed8cfa5 100644 --- a/catalogue/CatalogueTest.cpp +++ b/catalogue/CatalogueTest.cpp @@ -5056,7 +5056,7 @@ TEST_P(cta_catalogue_CatalogueTest, createTape_many_tapes) { // Effectively clone the tapes from m_tape1 but give each one its own VID for(uint64_t i = 1; i <= nbTapes; i++) { std::ostringstream vid; - vid << "vid" << i; + vid << "VID" << i; auto tape = m_tape1; tape.vid = vid.str(); @@ -5084,7 +5084,7 @@ TEST_P(cta_catalogue_CatalogueTest, createTape_many_tapes) { for(uint64_t i = 1; i <= nbTapes; i++) { std::ostringstream vid; - vid << "vid" << i; + vid << "VID" << i; auto vidAndTapeItor = vidToTape.find(vid.str()); ASSERT_NE(vidToTape.end(), vidAndTapeItor); @@ -5165,13 +5165,13 @@ TEST_P(cta_catalogue_CatalogueTest, createTape_many_tapes) { { catalogue::TapeSearchCriteria searchCriteria; - searchCriteria.vid = "vid1"; + searchCriteria.vid = "VID1"; const std::list<common::dataStructures::Tape> tapes = m_catalogue->getTapes(searchCriteria); ASSERT_EQ(1, tapes.size()); const std::map<std::string, common::dataStructures::Tape> vidToTape = tapeListToMap(tapes); ASSERT_EQ(1, vidToTape.size()); - ASSERT_EQ("vid1", vidToTape.begin()->first); - ASSERT_EQ("vid1", vidToTape.begin()->second.vid); + ASSERT_EQ("VID1", vidToTape.begin()->first); + ASSERT_EQ("VID1", vidToTape.begin()->second.vid); } { @@ -5272,7 +5272,7 @@ TEST_P(cta_catalogue_CatalogueTest, createTape_many_tapes) { { catalogue::TapeSearchCriteria searchCriteria; - searchCriteria.vid = "vid1"; + searchCriteria.vid = "VID1"; searchCriteria.logicalLibrary = m_tape1.logicalLibraryName; searchCriteria.tapePool = m_tape1.tapePoolName; searchCriteria.capacityInBytes = m_mediaType.capacityInBytes; @@ -5281,8 +5281,8 @@ TEST_P(cta_catalogue_CatalogueTest, createTape_many_tapes) { const std::list<common::dataStructures::Tape> tapes = m_catalogue->getTapes(searchCriteria); const std::map<std::string, common::dataStructures::Tape> vidToTape = tapeListToMap(tapes); ASSERT_EQ(1, vidToTape.size()); - ASSERT_EQ("vid1", vidToTape.begin()->first); - ASSERT_EQ("vid1", vidToTape.begin()->second.vid); + ASSERT_EQ("VID1", vidToTape.begin()->first); + ASSERT_EQ("VID1", vidToTape.begin()->second.vid); ASSERT_EQ(m_tape1.logicalLibraryName, vidToTape.begin()->second.logicalLibraryName); ASSERT_EQ(m_tape1.tapePoolName, vidToTape.begin()->second.tapePoolName); ASSERT_EQ(m_mediaType.capacityInBytes, vidToTape.begin()->second.capacityInBytes); @@ -5294,7 +5294,7 @@ TEST_P(cta_catalogue_CatalogueTest, createTape_many_tapes) { std::set<std::string> vids; for(uint64_t i = 1; i <= nbTapes; i++) { std::ostringstream vid; - vid << "vid" << i; + vid << "VID" << i; vids.insert(vid.str()); } @@ -5303,7 +5303,7 @@ TEST_P(cta_catalogue_CatalogueTest, createTape_many_tapes) { for(uint64_t i = 1; i <= nbTapes; i++) { std::ostringstream vid; - vid << "vid" << i; + vid << "VID" << i; auto vidAndTapeItor = vidToTape.find(vid.str()); ASSERT_NE(vidToTape.end(), vidAndTapeItor); diff --git a/catalogue/RdbmsCatalogue.cpp b/catalogue/RdbmsCatalogue.cpp index 77e9d13212..43bfd2095f 100644 --- a/catalogue/RdbmsCatalogue.cpp +++ b/catalogue/RdbmsCatalogue.cpp @@ -3706,6 +3706,10 @@ void RdbmsCatalogue::createTape( throw UserSpecifiedAnEmptyStringVid("Cannot create tape because the VID is an empty string"); } + if(!utils::isUpper(vid)) { + throw UserSpecifiedAnEmptyStringVid("Cannot create tape because the VID has non uppercase characters"); + } + if(mediaTypeName.empty()) { throw UserSpecifiedAnEmptyStringMediaType("Cannot create tape because the media type is an empty string"); } diff --git a/common/utils/utils.cpp b/common/utils/utils.cpp index 36698c46e5..83c7946824 100644 --- a/common/utils/utils.cpp +++ b/common/utils/utils.cpp @@ -776,6 +776,15 @@ void toUpper(std::string &str) { } } +//------------------------------------------------------------------------------ +// isUpper +//------------------------------------------------------------------------------ +bool isUpper(const std::string &str) { + std::string str_upper = str; + toUpper(str_upper); + return str_upper == str; +} + //------------------------------------------------------------------------------ // toLower //------------------------------------------------------------------------------ diff --git a/common/utils/utils.hpp b/common/utils/utils.hpp index 1b11cf2f10..8bcd2119af 100644 --- a/common/utils/utils.hpp +++ b/common/utils/utils.hpp @@ -319,6 +319,14 @@ namespace utils { */ void toUpper(std::string &str); + /** + * Checks if the specified string is uppercase. + * + * @param In/out parameter: The string to be checked. + * @return true if the string is uppercase. + */ + bool isUpper(const std::string &str); + /** * Converts the specified string to lowercase. * diff --git a/scheduler/GenericSchedulerTest.cpp b/scheduler/GenericSchedulerTest.cpp index 8d46afe61d..75c50f69fc 100644 --- a/scheduler/GenericSchedulerTest.cpp +++ b/scheduler/GenericSchedulerTest.cpp @@ -296,7 +296,7 @@ protected: const cta::common::dataStructures::SecurityIdentity s_adminOnAdminHost = { "admin1", "host1" }; const std::string s_tapePoolName = "TapePool"; const std::string s_libraryName = "TestLogicalLibrary"; - const std::string s_vid = "TestVid"; + const std::string s_vid = "TESTVID"; const std::string s_mediaType = "TestMediaType"; const std::string s_vendor = "TestVendor"; const std::string s_mountPolicyName = "mount_group"; @@ -465,7 +465,7 @@ TEST_P(SchedulerTest, archive_report_and_retrieve_new_file) { auto mi=osdb.getMountInfo(lc); ASSERT_EQ(1, mi->existingOrNextMounts.size()); ASSERT_EQ("TapePool", mi->existingOrNextMounts.front().tapePool); - ASSERT_EQ("TestVid", mi->existingOrNextMounts.front().vid); + ASSERT_EQ("TESTVID", mi->existingOrNextMounts.front().vid); std::unique_ptr<cta::ArchiveMount> archiveMount; archiveMount.reset(dynamic_cast<cta::ArchiveMount*>(mount.release())); ASSERT_NE(nullptr, archiveMount.get()); @@ -670,7 +670,7 @@ TEST_P(SchedulerTest, archive_report_and_retrieve_new_file_with_specific_mount_p auto mi=osdb.getMountInfo(lc); ASSERT_EQ(1, mi->existingOrNextMounts.size()); ASSERT_EQ("TapePool", mi->existingOrNextMounts.front().tapePool); - ASSERT_EQ("TestVid", mi->existingOrNextMounts.front().vid); + ASSERT_EQ("TESTVID", mi->existingOrNextMounts.front().vid); std::unique_ptr<cta::ArchiveMount> archiveMount; archiveMount.reset(dynamic_cast<cta::ArchiveMount*>(mount.release())); ASSERT_NE(nullptr, archiveMount.get()); @@ -968,7 +968,7 @@ TEST_P(SchedulerTest, archive_report_and_retrieve_new_dual_copy_file) { ASSERT_EQ(libraryComment, libraries.front().comment); } - const std::string copy1TapeVid = "copy_1_tape"; + const std::string copy1TapeVid = "COPY_1_TAPE"; { using namespace cta; @@ -1043,7 +1043,7 @@ TEST_P(SchedulerTest, archive_report_and_retrieve_new_dual_copy_file) { // Create the environment for the migration of copy 2 to happen (library + // tape) catalogue.setLogicalLibraryDisabled(s_adminOnAdminHost,s_libraryName,true); - const std::string copy2TapeVid = "copy_2_tape"; + const std::string copy2TapeVid = "COPY_2_TAPE"; { using namespace cta; @@ -1315,7 +1315,7 @@ TEST_P(SchedulerTest, archive_and_retrieve_failure) { auto mi=osdb.getMountInfo(lc); ASSERT_EQ(1, mi->existingOrNextMounts.size()); ASSERT_EQ("TapePool", mi->existingOrNextMounts.front().tapePool); - ASSERT_EQ("TestVid", mi->existingOrNextMounts.front().vid); + ASSERT_EQ("TESTVID", mi->existingOrNextMounts.front().vid); std::unique_ptr<cta::ArchiveMount> archiveMount; archiveMount.reset(dynamic_cast<cta::ArchiveMount*>(mount.release())); ASSERT_NE(nullptr, archiveMount.get()); @@ -1546,7 +1546,7 @@ TEST_P(SchedulerTest, archive_and_retrieve_report_failure) { auto mi=osdb.getMountInfo(lc); ASSERT_EQ(1, mi->existingOrNextMounts.size()); ASSERT_EQ("TapePool", mi->existingOrNextMounts.front().tapePool); - ASSERT_EQ("TestVid", mi->existingOrNextMounts.front().vid); + ASSERT_EQ("TESTVID", mi->existingOrNextMounts.front().vid); std::unique_ptr<cta::ArchiveMount> archiveMount; archiveMount.reset(dynamic_cast<cta::ArchiveMount*>(mount.release())); ASSERT_NE(nullptr, archiveMount.get()); diff --git a/scheduler/SchedulerTest.cpp b/scheduler/SchedulerTest.cpp index f733054700..6189cfc9d4 100644 --- a/scheduler/SchedulerTest.cpp +++ b/scheduler/SchedulerTest.cpp @@ -341,7 +341,7 @@ protected: const cta::common::dataStructures::SecurityIdentity s_adminOnAdminHost = { "admin1", "host1" }; const std::string s_tapePoolName = "TapePool"; const std::string s_libraryName = "TestLogicalLibrary"; - const std::string s_vid = "TestVid"; + const std::string s_vid = "TESTVID"; const std::string s_mediaType = "TestMediaType"; const std::string s_vendor = "TestVendor"; const std::string s_mountPolicyName = "mount_group"; @@ -582,7 +582,7 @@ TEST_P(SchedulerTest, archive_report_and_retrieve_new_file) { auto mi=osdb.getMountInfo(lc); ASSERT_EQ(1, mi->existingOrNextMounts.size()); ASSERT_EQ("TapePool", mi->existingOrNextMounts.front().tapePool); - ASSERT_EQ("TestVid", mi->existingOrNextMounts.front().vid); + ASSERT_EQ("TESTVID", mi->existingOrNextMounts.front().vid); std::unique_ptr<cta::ArchiveMount> archiveMount; archiveMount.reset(dynamic_cast<cta::ArchiveMount*>(mount.release())); ASSERT_NE(nullptr, archiveMount.get()); @@ -787,7 +787,7 @@ TEST_P(SchedulerTest, archive_report_and_retrieve_new_file_with_specific_mount_p auto mi=osdb.getMountInfo(lc); ASSERT_EQ(1, mi->existingOrNextMounts.size()); ASSERT_EQ("TapePool", mi->existingOrNextMounts.front().tapePool); - ASSERT_EQ("TestVid", mi->existingOrNextMounts.front().vid); + ASSERT_EQ("TESTVID", mi->existingOrNextMounts.front().vid); std::unique_ptr<cta::ArchiveMount> archiveMount; archiveMount.reset(dynamic_cast<cta::ArchiveMount*>(mount.release())); ASSERT_NE(nullptr, archiveMount.get()); @@ -1085,7 +1085,7 @@ TEST_P(SchedulerTest, archive_report_and_retrieve_new_dual_copy_file) { ASSERT_EQ(libraryComment, libraries.front().comment); } - const std::string copy1TapeVid = "copy_1_tape"; + const std::string copy1TapeVid = "COPY_1_TAPE"; { using namespace cta; @@ -1160,7 +1160,7 @@ TEST_P(SchedulerTest, archive_report_and_retrieve_new_dual_copy_file) { // Create the environment for the migration of copy 2 to happen (library + // tape) catalogue.setLogicalLibraryDisabled(s_adminOnAdminHost,s_libraryName,true); - const std::string copy2TapeVid = "copy_2_tape"; + const std::string copy2TapeVid = "COPY_2_TAPE"; { using namespace cta; @@ -1432,7 +1432,7 @@ TEST_P(SchedulerTest, archive_and_retrieve_failure) { auto mi=osdb.getMountInfo(lc); ASSERT_EQ(1, mi->existingOrNextMounts.size()); ASSERT_EQ("TapePool", mi->existingOrNextMounts.front().tapePool); - ASSERT_EQ("TestVid", mi->existingOrNextMounts.front().vid); + ASSERT_EQ("TESTVID", mi->existingOrNextMounts.front().vid); std::unique_ptr<cta::ArchiveMount> archiveMount; archiveMount.reset(dynamic_cast<cta::ArchiveMount*>(mount.release())); ASSERT_NE(nullptr, archiveMount.get()); @@ -1682,7 +1682,7 @@ TEST_P(SchedulerTest, archive_and_retrieve_report_failure) { auto mi=osdb.getMountInfo(lc); ASSERT_EQ(1, mi->existingOrNextMounts.size()); ASSERT_EQ("TapePool", mi->existingOrNextMounts.front().tapePool); - ASSERT_EQ("TestVid", mi->existingOrNextMounts.front().vid); + ASSERT_EQ("TESTVID", mi->existingOrNextMounts.front().vid); std::unique_ptr<cta::ArchiveMount> archiveMount; archiveMount.reset(dynamic_cast<cta::ArchiveMount*>(mount.release())); ASSERT_NE(nullptr, archiveMount.get()); @@ -2031,7 +2031,7 @@ TEST_P(SchedulerTest, repack) { common::dataStructures::SecurityIdentity cliId; cliId.host = "host"; cliId.username = s_userName; - std::string tape1 = "Tape"; + std::string tape1 = "TAPE"; { auto tape = getDefaultTape(); @@ -2063,7 +2063,7 @@ TEST_P(SchedulerTest, repack) { scheduler.cancelRepack(cliId, tape1, lc); ASSERT_EQ(0, scheduler.getRepacks().size()); // Recreate a repack and get it moved to ToExpand - std::string tape2 = "Tape2"; + std::string tape2 = "TAPE2"; { auto tape = getDefaultTape(); catalogue.createTape(s_adminOnAdminHost, tape); @@ -2110,7 +2110,7 @@ TEST_P(SchedulerTest, getNextRepackRequestToExpand) { common::dataStructures::SecurityIdentity cliId; cliId.host = "host"; cliId.username = s_userName; - std::string tape1 = "Tape"; + std::string tape1 = "TAPE"; { auto tape = getDefaultTape(); tape.vid = tape1; @@ -2125,7 +2125,7 @@ TEST_P(SchedulerTest, getNextRepackRequestToExpand) { common::dataStructures::MountPolicy::s_defaultMountPolicyForRepack,s_defaultRepackNoRecall); scheduler.queueRepack(cliId, qrr, lc); - std::string tape2 = "Tape2"; + std::string tape2 = "TAPE2"; { auto tape = getDefaultTape(); @@ -2788,7 +2788,7 @@ TEST_P(SchedulerTest, expandRepackRequestArchiveSuccess) { } //Create a repack destination tape - std::string vidDestination = "vidDestination"; + std::string vidDestination = "VIDDESTINATION"; { auto tape = getDefaultTape(); @@ -3047,7 +3047,7 @@ TEST_P(SchedulerTest, expandRepackRequestArchiveFailed) { } //Create a repack destination tape - std::string vidDestinationRepack = "vidDestinationRepack"; + std::string vidDestinationRepack = "VIDDESTINATIONREPACK"; { auto tape = getDefaultTape(); tape.vid = vidDestinationRepack; @@ -4267,7 +4267,7 @@ TEST_P(SchedulerTest, expandRepackRequestAddCopiesOnly) { catalogue.createLogicalLibrary(admin, s_libraryName, logicalLibraryIsDisabled, "Create logical library"); //Create the source tape - std::string vid = "vidSource"; + std::string vid = "VIDSOURCE"; { auto tape = getDefaultTape(); tape.vid = vid; @@ -4297,7 +4297,7 @@ TEST_P(SchedulerTest, expandRepackRequestAddCopiesOnly) { catalogue.createArchiveRoute(admin,storageClass.name,3,tapepool3Name,"ArchiveRoute3"); //Create two other destinationTape - std::string vidDestination1 = "vidDestination1"; + std::string vidDestination1 = "VIDDESTINATION1"; { auto tape = getDefaultTape(); tape.vid = vidDestination1; @@ -4305,7 +4305,7 @@ TEST_P(SchedulerTest, expandRepackRequestAddCopiesOnly) { catalogue.createTape(s_adminOnAdminHost, tape); } - std::string vidDestination2 = "vidDestination2"; + std::string vidDestination2 = "VIDDESTINATION2"; { auto tape = getDefaultTape(); tape.vid = vidDestination2; @@ -4519,7 +4519,7 @@ TEST_P(SchedulerTest, expandRepackRequestShouldFailIfArchiveRouteMissing) { catalogue.createLogicalLibrary(admin, s_libraryName, logicalLibraryIsDisabled, "Create logical library"); //Create the source tape - std::string vidCopyNb1 = "vidSource"; + std::string vidCopyNb1 = "VIDSOURCE"; { auto tape = getDefaultTape(); tape.vid = vidCopyNb1; @@ -4545,7 +4545,7 @@ TEST_P(SchedulerTest, expandRepackRequestShouldFailIfArchiveRouteMissing) { catalogue.createArchiveRoute(admin,storageClass.name,2,tapepool2Name,"ArchiveRoute3"); //Create two other destinationTape - std::string vidCopyNb2_source = "vidCopyNb2-source"; + std::string vidCopyNb2_source = "VIDCOPYNB2_SOURCE"; { auto tape = getDefaultTape(); tape.vid = vidCopyNb2_source; @@ -4555,7 +4555,7 @@ TEST_P(SchedulerTest, expandRepackRequestShouldFailIfArchiveRouteMissing) { catalogue.createTape(s_adminOnAdminHost, tape); } - std::string vidCopyNb2_destination = "vidCopyNb2-destination"; + std::string vidCopyNb2_destination = "VIDCOPYNB2_DESTINATION"; { auto tape = getDefaultTape(); tape.vid = vidCopyNb2_destination; @@ -4701,7 +4701,7 @@ TEST_P(SchedulerTest, expandRepackRequestMoveAndAddCopies){ catalogue.createLogicalLibrary(admin, s_libraryName, logicalLibraryIsDisabled, "Create logical library"); //Create the source tape - std::string vid = "vidSource"; + std::string vid = "VIDSOURCE"; { auto tape = getDefaultTape(); tape.vid = vid; @@ -4731,7 +4731,7 @@ TEST_P(SchedulerTest, expandRepackRequestMoveAndAddCopies){ catalogue.createArchiveRoute(admin,storageClass.name,3,tapepool3Name,"ArchiveRoute3"); //Create two other destinationTape and one for the move workflow - std::string vidDestination1 = "vidDestination1"; + std::string vidDestination1 = "VIDDESTINATION1"; { auto tape = getDefaultTape(); tape.vid = vidDestination1; @@ -4739,7 +4739,7 @@ TEST_P(SchedulerTest, expandRepackRequestMoveAndAddCopies){ catalogue.createTape(s_adminOnAdminHost, tape); } - std::string vidDestination2 = "vidDestination2"; + std::string vidDestination2 = "VIDDESTINATION2"; { auto tape = getDefaultTape(); tape.vid = vidDestination2; @@ -4748,7 +4748,7 @@ TEST_P(SchedulerTest, expandRepackRequestMoveAndAddCopies){ catalogue.createTape(s_adminOnAdminHost, tape); } - std::string vidMove = "vidMove"; + std::string vidMove = "VIDMOVE"; { auto tape = getDefaultTape(); tape.vid = vidMove; @@ -4992,7 +4992,7 @@ TEST_P(SchedulerTest, cancelRepackRequest) { catalogue.createTape(s_adminOnAdminHost, tape); } //Create a repack destination tape - std::string vidDestination = "vidDestination"; + std::string vidDestination = "VIDDESTINATION"; { auto tape = getDefaultTape(); tape.vid = vidDestination; @@ -6032,7 +6032,7 @@ TEST_P(SchedulerTest, retrieveArchiveAllTypesMaxDrivesVoInFlightChangeScheduleMo catalogue.createTape(s_adminOnAdminHost, tape1); //Two tapes for ArchiveForUser and ArchiveForRepack mounts - std::string vid2 = "vid_2"; + std::string vid2 = "VID_2"; auto tape2 = tape1; tape2.vid = vid2; catalogue.createTape(s_adminOnAdminHost, tape2); @@ -6042,7 +6042,7 @@ TEST_P(SchedulerTest, retrieveArchiveAllTypesMaxDrivesVoInFlightChangeScheduleMo catalogue.createTapePool(s_adminOnAdminHost,newTapepool,s_vo,1,false,std::nullopt,"Test"); //Create the third tape in the new tapepool - std::string vid3 = "vid_3"; + std::string vid3 = "VID_3"; auto tape3 = tape1; tape3.vid = vid3; tape3.tapePoolName = newTapepool; diff --git a/tapeserver/castor/tape/tapeserver/daemon/DataTransferSessionTest.cpp b/tapeserver/castor/tape/tapeserver/daemon/DataTransferSessionTest.cpp index 86716dc2e9..165586a1b1 100644 --- a/tapeserver/castor/tape/tapeserver/daemon/DataTransferSessionTest.cpp +++ b/tapeserver/castor/tape/tapeserver/daemon/DataTransferSessionTest.cpp @@ -455,7 +455,7 @@ protected: const cta::common::dataStructures::SecurityIdentity s_adminOnAdminHost = {"admin1", "host1"}; const std::string s_tapePoolName = "TestTapePool"; const std::string s_libraryName = "TestLogicalLibrary"; - const std::string s_vid = "TstVid"; // We really need size <= 6 characters due to tape label format. + const std::string s_vid = "TSTVID"; // We really need size <= 6 characters due to tape label format. const std::string s_mediaType = "LTO7M"; const std::string s_vendor = "TestVendor"; //TempFile m_tempSqliteFile; @@ -636,7 +636,7 @@ TEST_P(DataTransferSessionTest, DataTransferSessionGooddayRecall) { // 10) Check logs std::string logToCheck = logger.getLog(); logToCheck += ""; - ASSERT_NE(std::string::npos, logToCheck.find("MSG=\"Tape session started for read\" thread=\"TapeRead\" tapeDrive=\"T10D6116\" tapeVid=\"TstVid\" " + ASSERT_NE(std::string::npos, logToCheck.find("MSG=\"Tape session started for read\" thread=\"TapeRead\" tapeDrive=\"T10D6116\" tapeVid=\"TSTVID\" " "mountId=\"1\" vo=\"vo\" mediaType=\"LTO7M\" tapePool=\"TestTapePool\" " "logicalLibrary=\"TestLogicalLibrary\" mountType=\"Retrieve\" labelFormat=\"0000\" vendor=\"TestVendor\" " "capacityInBytes=\"12345678\"")); @@ -832,7 +832,7 @@ TEST_P(DataTransferSessionTest, DataTransferSessionWrongChecksumRecall) { // 10) Check logs std::string logToCheck = logger.getLog(); logToCheck += ""; - ASSERT_NE(std::string::npos,logToCheck.find("MSG=\"Tape session started for read\" thread=\"TapeRead\" tapeDrive=\"T10D6116\" tapeVid=\"TstVid\" " + ASSERT_NE(std::string::npos,logToCheck.find("MSG=\"Tape session started for read\" thread=\"TapeRead\" tapeDrive=\"T10D6116\" tapeVid=\"TSTVID\" " "mountId=\"1\" vo=\"vo\" mediaType=\"LTO7M\" tapePool=\"TestTapePool\" " "logicalLibrary=\"TestLogicalLibrary\" mountType=\"Retrieve\" labelFormat=\"0000\" vendor=\"TestVendor\" " "capacityInBytes=\"12345678\"")); @@ -3146,7 +3146,7 @@ TEST_P(DataTransferSessionTest, DataTransferSessionTapeFullMigration) { std::string logToCheck = logger.getLog(); logToCheck += ""; ASSERT_NE(std::string::npos, - logToCheck.find("MSG=\"Tape session started for write\" thread=\"TapeWrite\" tapeDrive=\"T10D6116\" tapeVid=\"TstVid\" " + logToCheck.find("MSG=\"Tape session started for write\" thread=\"TapeWrite\" tapeDrive=\"T10D6116\" tapeVid=\"TSTVID\" " "mountId=\"1\" vo=\"vo\" mediaType=\"LTO7M\" tapePool=\"TestTapePool\" logicalLibrary=\"TestLogicalLibrary\" " "mountType=\"ArchiveForUser\" vendor=\"TestVendor\" capacityInBytes=\"12345678\"")); ASSERT_NE(std::string::npos, logToCheck.find("firmwareVersion=\"123A\" serialNumber=\"123456\" " diff --git a/tapeserver/tapelabel/TapeLabelCmdLineArgs.cpp b/tapeserver/tapelabel/TapeLabelCmdLineArgs.cpp index c95b29feaf..4683e789ce 100644 --- a/tapeserver/tapelabel/TapeLabelCmdLineArgs.cpp +++ b/tapeserver/tapelabel/TapeLabelCmdLineArgs.cpp @@ -57,21 +57,27 @@ TapeLabelCmdLineArgs::TapeLabelCmdLineArgs(const int argc, char *const *const ar case 'v': if (strlen(optarg) > CA_MAXVIDLEN) { exception::CommandLineNotParsed ex; - ex.getMessage() << "The -" << (char)opt << " option too big"; + ex.getMessage() << "The -" << static_cast<char>(opt) << " option too big"; + throw ex; + } + m_vid = std::string(optarg); + if (!utils::isUpper(m_vid)) { + exception::CommandLineNotParsed ex; + ex.getMessage() << "The -" << static_cast<char>(opt) << " option must only contain uppercase alphanumeric characters"; throw ex; - } else { - m_vid = std::string(optarg); - utils::toUpper(m_vid); } break; case 'o': if (strlen(optarg) > CA_MAXVIDLEN) { exception::CommandLineNotParsed ex; - ex.getMessage() << "The -" << (char)opt << " option too big"; + ex.getMessage() << "The -" << static_cast<char>(opt) << " option too big"; + throw ex; + } + m_oldLabel = std::string(optarg); + if (!utils::isUpper(m_vid)) { + exception::CommandLineNotParsed ex; + ex.getMessage() << "The -" << static_cast<char>(opt) << " option must only contain uppercase alphanumeric characters"; throw ex; - } else { - m_oldLabel = std::string(optarg); - utils::toUpper(m_oldLabel); } break; case 't': @@ -107,7 +113,7 @@ TapeLabelCmdLineArgs::TapeLabelCmdLineArgs(const int argc, char *const *const ar case ':': // Missing parameter { exception::CommandLineNotParsed ex; - ex.getMessage() << "The -" << (char)optopt << " option requires a parameter"; + ex.getMessage() << "The -" << static_cast<char>(optopt) << " option requires a parameter"; throw ex; } case '?': // Unknown option @@ -116,7 +122,7 @@ TapeLabelCmdLineArgs::TapeLabelCmdLineArgs(const int argc, char *const *const ar if(0 == optopt) { ex.getMessage() << "Unknown command-line option"; } else { - ex.getMessage() << "Unknown command-line option: -" << (char)optopt; + ex.getMessage() << "Unknown command-line option: -" << static_cast<char>(optopt); } throw ex; } @@ -125,7 +131,7 @@ TapeLabelCmdLineArgs::TapeLabelCmdLineArgs(const int argc, char *const *const ar exception::CommandLineNotParsed ex; ex.getMessage() << "getopt_long returned the following unknown value: 0x" << - std::hex << (int)opt; + std::hex << static_cast<int>(opt); throw ex; } } // switch(opt) -- GitLab