From f86a488231dc9a629d44a9f6aa6bfc9aa0eb57b4 Mon Sep 17 00:00:00 2001 From: Squire Date: Fri, 17 Jun 2011 11:05:31 -0700 Subject: CHOP-662 - initial fix to problems with regex characters in Group names causing the LLDirIterator to crash the viewer when opening chat logs --- indra/llvfs/CMakeLists.txt | 1 + indra/llvfs/lldiriterator.cpp | 21 ++++++++--- indra/llvfs/tests/lldiriterator_test.cpp | 62 ++++++++++++++++++++++++++++++++ indra/newview/lllogchat.cpp | 2 +- 4 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 indra/llvfs/tests/lldiriterator_test.cpp diff --git a/indra/llvfs/CMakeLists.txt b/indra/llvfs/CMakeLists.txt index b6d1ce61e5..f89da4d6c5 100644 --- a/indra/llvfs/CMakeLists.txt +++ b/indra/llvfs/CMakeLists.txt @@ -80,6 +80,7 @@ if (LL_TESTS) # UNIT TESTS SET(llvfs_TEST_SOURCE_FILES # none so far + lldiriterator.cpp ) LL_ADD_PROJECT_UNIT_TESTS(llvfs "${llvfs_TEST_SOURCE_FILES}") diff --git a/indra/llvfs/lldiriterator.cpp b/indra/llvfs/lldiriterator.cpp index 041436ed92..3b132862ad 100644 --- a/indra/llvfs/lldiriterator.cpp +++ b/indra/llvfs/lldiriterator.cpp @@ -121,6 +121,14 @@ bool LLDirIterator::Impl::next(std::string &fname) return found; } +/* +converts the incoming glob into a regex. This involves +converting incoming glob expressions to regex equivilents and +at the same time, escaping any regex meaningful characters which +do not have glob meaning, i.e. + .()+|^$ +in the input +*/ std::string glob_to_regex(const std::string& glob) { std::string regex; @@ -135,9 +143,6 @@ std::string glob_to_regex(const std::string& glob) switch (c) { - case '.': - regex+="\\."; - break; case '*': if (glob.begin() == i) { @@ -170,7 +175,15 @@ std::string glob_to_regex(const std::string& glob) case '!': regex+= square_brace_open ? '^' : c; break; - default: + case '.': // This collection have different regex meaning + case '^': // And so need escaping + case '(': + case ')': + case '+': + case '|': + case '$': + regex+='\\'; + default: regex+=c; break; } diff --git a/indra/llvfs/tests/lldiriterator_test.cpp b/indra/llvfs/tests/lldiriterator_test.cpp new file mode 100644 index 0000000000..7c2cd49e41 --- /dev/null +++ b/indra/llvfs/tests/lldiriterator_test.cpp @@ -0,0 +1,62 @@ +/** + * @file lldiriterator_test.cpp + * @date 2011-06 + * @brief LLDirIterator test cases. + * + * $LicenseInfo:firstyear=2011&license=viewerlgpl$ + * Second Life Viewer Source Code + * Copyright (C) 2011, Linden Research, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; + * version 2.1 of the License only., + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * Linden Research, Inc., 945 Battery Street, San Francisco, CA 94111 USA + * $/LicenseInfo$ + */ + +#include "linden_common.h" +#include "lltut.h" +#include "../lldiriterator.h" + + +namespace tut +{ + + struct LLDirIteratorFixture + { + LLDirIteratorFixture() + { + } + }; + typedef test_group LLDirIteratorTest_factory; + typedef LLDirIteratorTest_factory::object LLDirIteratorTest_t; + LLDirIteratorTest_factory tf("LLDirIterator"); + + /* + CHOP-662 was originally introduced to deal with crashes deleting files from + a directory (VWR-25500). However, this introduced a crash looking for + old chat logs as the glob_to_regex function in lldiriterator wasn't escaping lots of regexp characters + */ + void test_chop_662(void) + { + LLDirIterator iter(".","+bad-group-name]+??-??.*"); + } + + template<> template<> + void LLDirIteratorTest_t::test<1>() + { + test_chop_662(); + } + +} \ No newline at end of file diff --git a/indra/newview/lllogchat.cpp b/indra/newview/lllogchat.cpp index efc4e23838..ebb5912ace 100644 --- a/indra/newview/lllogchat.cpp +++ b/indra/newview/lllogchat.cpp @@ -230,7 +230,7 @@ std::string LLLogChat::makeLogFileName(std::string filename) std::string LLLogChat::cleanFileName(std::string filename) { - std::string invalidChars = "\"\'\\/?*:.<>|"; + std::string invalidChars = "\"\'\\/?*:.<>|[]{}~"; // Cannot match glob or illegal filename chars std::string::size_type position = filename.find_first_of(invalidChars); while (position != filename.npos) { -- cgit v1.2.3 From 29f1b466e66fd5a98926ac7318287c22f74e32bf Mon Sep 17 00:00:00 2001 From: Squire Date: Fri, 17 Jun 2011 14:13:01 -0700 Subject: COP-662 Added newline at end of file for Linux build --- indra/llvfs/tests/lldiriterator_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/indra/llvfs/tests/lldiriterator_test.cpp b/indra/llvfs/tests/lldiriterator_test.cpp index 7c2cd49e41..0e7017f59f 100644 --- a/indra/llvfs/tests/lldiriterator_test.cpp +++ b/indra/llvfs/tests/lldiriterator_test.cpp @@ -59,4 +59,4 @@ namespace tut test_chop_662(); } -} \ No newline at end of file +} -- cgit v1.2.3 From 3a25753bcf46cc8d36bcf3d3bf21d9d0d94dbd34 Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Fri, 17 Jun 2011 16:16:01 -0700 Subject: STORM-1368 Code more defensively around notecard upload. --- indra/newview/llpreviewnotecard.cpp | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/indra/newview/llpreviewnotecard.cpp b/indra/newview/llpreviewnotecard.cpp index 9f3ee6ac5d..4974dde282 100644 --- a/indra/newview/llpreviewnotecard.cpp +++ b/indra/newview/llpreviewnotecard.cpp @@ -401,15 +401,14 @@ struct LLSaveNotecardInfo bool LLPreviewNotecard::saveIfNeeded(LLInventoryItem* copyitem) { - if(!gAssetStorage) + LLViewerTextEditor* editor = getChild("Notecard Editor"); + + if(!editor) { - llwarns << "Not connected to an asset storage system." << llendl; + llwarns << "Cannot get handle to the notecard editor." << llendl; return false; } - - LLViewerTextEditor* editor = getChild("Notecard Editor"); - if(!editor->isPristine()) { // We need to update the asset information @@ -436,8 +435,15 @@ bool LLPreviewNotecard::saveIfNeeded(LLInventoryItem* copyitem) // save it out to database if (item) { - std::string agent_url = gAgent.getRegion()->getCapability("UpdateNotecardAgentInventory"); - std::string task_url = gAgent.getRegion()->getCapability("UpdateNotecardTaskInventory"); + const LLViewerRegion* region = gAgent.getRegion(); + if (!region) + { + llwarns << "Not connected to a region, cannot save notecard." << llendl; + return false; + } + std::string agent_url = region->getCapability("UpdateNotecardAgentInventory"); + std::string task_url = region->getCapability("UpdateNotecardTaskInventory"); + if (mObjectUUID.isNull() && !agent_url.empty()) { // Saving into agent inventory @@ -472,6 +478,11 @@ bool LLPreviewNotecard::saveIfNeeded(LLInventoryItem* copyitem) (void*)info, FALSE); } + else // !gAssetStorage + { + llwarns << "Not connected to an asset storage system." << llendl; + return false; + } } } return true; -- cgit v1.2.3 From c0971cd0a196dfcf9bd1be9fc2c77c817438c24a Mon Sep 17 00:00:00 2001 From: Squire Date: Fri, 17 Jun 2011 16:46:31 -0700 Subject: CHOP-662 Added booost library to unittest for lldiriterator.cpp --- indra/llvfs/CMakeLists.txt | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/indra/llvfs/CMakeLists.txt b/indra/llvfs/CMakeLists.txt index f89da4d6c5..2c581cf8d6 100644 --- a/indra/llvfs/CMakeLists.txt +++ b/indra/llvfs/CMakeLists.txt @@ -62,11 +62,15 @@ list(APPEND llvfs_SOURCE_FILES ${llvfs_HEADER_FILES}) add_library (llvfs ${llvfs_SOURCE_FILES}) -target_link_libraries(llvfs +set(vfs_BOOST_LIBRARIES ${BOOST_FILESYSTEM_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ) +target_link_libraries(llvfs + ${vfs_BOOST_LIBRARIES} + ) + if (DARWIN) include(CMakeFindFrameworks) find_library(CARBON_LIBRARY Carbon) @@ -76,16 +80,21 @@ endif (DARWIN) # Add tests if (LL_TESTS) - include(LLAddBuildTest) - # UNIT TESTS - SET(llvfs_TEST_SOURCE_FILES - # none so far - lldiriterator.cpp - ) - LL_ADD_PROJECT_UNIT_TESTS(llvfs "${llvfs_TEST_SOURCE_FILES}") - - # INTEGRATION TESTS - set(test_libs llmath llcommon llvfs ${LLCOMMON_LIBRARIES} ${WINDOWS_LIBRARIES}) - # TODO: Some of these need refactoring to be proper Unit tests rather than Integration tests. - LL_ADD_INTEGRATION_TEST(lldir "" "${test_libs}") + include(LLAddBuildTest) + # UNIT TESTS + SET(llvfs_TEST_SOURCE_FILES + lldiriterator.cpp + ) + + set_source_files_properties(lldiriterator.cpp + PROPERTIES + LL_TEST_ADDITIONAL_LIBRARIES "${vfs_BOOST_LIBRARIES}" + ) + LL_ADD_PROJECT_UNIT_TESTS(llvfs "${llvfs_TEST_SOURCE_FILES}") + + # INTEGRATION TESTS + set(test_libs llmath llcommon llvfs ${LLCOMMON_LIBRARIES} ${WINDOWS_LIBRARIES}) + + # TODO: Some of these need refactoring to be proper Unit tests rather than Integration tests. + LL_ADD_INTEGRATION_TEST(lldir "" "${test_libs}") endif (LL_TESTS) -- cgit v1.2.3 From 306759835d8aed3a041a59dafaa20a8c708e2dd4 Mon Sep 17 00:00:00 2001 From: Squire Date: Mon, 20 Jun 2011 11:07:17 -0700 Subject: Added more tests to cover the common examples in the crash logs --- indra/llvfs/tests/lldiriterator_test.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/indra/llvfs/tests/lldiriterator_test.cpp b/indra/llvfs/tests/lldiriterator_test.cpp index 0e7017f59f..aacc5a616a 100644 --- a/indra/llvfs/tests/lldiriterator_test.cpp +++ b/indra/llvfs/tests/lldiriterator_test.cpp @@ -50,7 +50,10 @@ namespace tut */ void test_chop_662(void) { + // Check a selection of bad group names from the crash reports LLDirIterator iter(".","+bad-group-name]+??-??.*"); + LLDirIterator iter(".","))--@---bad-group-name2((??-??.*\.txt"); + LLDirIterator iter(".","__^v--x)Cuide d sua vida(x--v^__??-??.*"); } template<> template<> -- cgit v1.2.3 From f02dc7e1b6c26d897628617a870f250004febee0 Mon Sep 17 00:00:00 2001 From: Squire Date: Mon, 20 Jun 2011 12:17:25 -0700 Subject: CHOP-662 Coupl of fixes to extended unit-test --- indra/llvfs/tests/lldiriterator_test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/indra/llvfs/tests/lldiriterator_test.cpp b/indra/llvfs/tests/lldiriterator_test.cpp index aacc5a616a..505d86faa7 100644 --- a/indra/llvfs/tests/lldiriterator_test.cpp +++ b/indra/llvfs/tests/lldiriterator_test.cpp @@ -52,8 +52,8 @@ namespace tut { // Check a selection of bad group names from the crash reports LLDirIterator iter(".","+bad-group-name]+??-??.*"); - LLDirIterator iter(".","))--@---bad-group-name2((??-??.*\.txt"); - LLDirIterator iter(".","__^v--x)Cuide d sua vida(x--v^__??-??.*"); + LLDirIterator iter1(".","))--@---bad-group-name2((??-??.*\\.txt"); + LLDirIterator iter2(".","__^v--x)Cuide d sua vida(x--v^__??-??.*"); } template<> template<> -- cgit v1.2.3 From a975df88f03a301bfaf4ba0b9015e46db02d399f Mon Sep 17 00:00:00 2001 From: Squire Date: Mon, 20 Jun 2011 16:30:48 -0700 Subject: Added change to contributions.txt to get code review to work --- doc/contributions.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/contributions.txt b/doc/contributions.txt index b744f4db3e..de917d4f18 100644 --- a/doc/contributions.txt +++ b/doc/contributions.txt @@ -759,6 +759,8 @@ SpacedOut Frye VWR-1823 Sporked Friis VWR-4903 +Squire Linden + CHOP-662 Stevex Janus VWR-1182 Still Defiant -- cgit v1.2.3 From 14105610c7fb295a16b3b2a88014004c62ffa265 Mon Sep 17 00:00:00 2001 From: Squire Date: Tue, 21 Jun 2011 20:46:32 -0700 Subject: CHOP-662 cleanup in response to code review. --- doc/contributions.txt | 2 -- indra/llvfs/lldiriterator.cpp | 14 +++++++------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/doc/contributions.txt b/doc/contributions.txt index de917d4f18..b744f4db3e 100644 --- a/doc/contributions.txt +++ b/doc/contributions.txt @@ -759,8 +759,6 @@ SpacedOut Frye VWR-1823 Sporked Friis VWR-4903 -Squire Linden - CHOP-662 Stevex Janus VWR-1182 Still Defiant diff --git a/indra/llvfs/lldiriterator.cpp b/indra/llvfs/lldiriterator.cpp index 3b132862ad..25550321f0 100644 --- a/indra/llvfs/lldiriterator.cpp +++ b/indra/llvfs/lldiriterator.cpp @@ -121,13 +121,13 @@ bool LLDirIterator::Impl::next(std::string &fname) return found; } -/* -converts the incoming glob into a regex. This involves +/** +Converts the incoming glob into a regex. This involves converting incoming glob expressions to regex equivilents and at the same time, escaping any regex meaningful characters which do not have glob meaning, i.e. .()+|^$ -in the input +in the input. */ std::string glob_to_regex(const std::string& glob) { @@ -176,15 +176,15 @@ std::string glob_to_regex(const std::string& glob) regex+= square_brace_open ? '^' : c; break; case '.': // This collection have different regex meaning - case '^': // And so need escaping + case '^': // and so need escaping. case '(': case ')': case '+': case '|': case '$': - regex+='\\'; - default: - regex+=c; + regex += '\\'; + default: + regex += c; break; } -- cgit v1.2.3