From f3a1f0f9f25a27afe22d38e943af98b389870bdf Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Sat, 15 Jul 2006 01:54:49 +0000
Subject: [PATCH] GroupText.t: fix number of tests Session/Id.pm: add a method
 to validate GUIDs, change s/// to tr/// for efficiency. Session.pm: change
 open to use new Id.pm validation method Scratch.t: Change test from ok to is
 so that it tells you what the failing ID is. Id.t: Add validation tests in
 addition to uniqueness tests.  Fix the uniqueness test so that it works.  Add
 tests to check the new validation method

---
 docs/changelog/7.x.x.txt |  2 ++
 lib/WebGUI/Session.pm    |  2 +-
 lib/WebGUI/Session/Id.pm | 16 +++++++++++--
 t/Macro/GroupText.t      |  2 +-
 t/Session/Id.t           | 49 +++++++++++++++++++++++++++++++++++++---
 t/Session/Scratch.t      |  2 +-
 6 files changed, 65 insertions(+), 8 deletions(-)

diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt
index ddb5b8ae9..0ccabde2e 100644
--- a/docs/changelog/7.x.x.txt
+++ b/docs/changelog/7.x.x.txt
@@ -13,6 +13,8 @@
  - Added an unsubscribe link to the messages generated by collaboration
    subscriptions per the laws in various countries.
  - fix: MultiSearch
+ - fix: Unable to duplicate existing Session Id
+
 
 7.0.1
  - fix: User profile field "Department" needs i18n 
diff --git a/lib/WebGUI/Session.pm b/lib/WebGUI/Session.pm
index 6e5b6e834..39c57c45e 100644
--- a/lib/WebGUI/Session.pm
+++ b/lib/WebGUI/Session.pm
@@ -370,7 +370,7 @@ sub open {
 	bless $self , $class;
 	$self->{_request} = Apache2::Request->new($request) if (defined $request);
 	my $sessionId = shift || $self->http->getCookies->{"wgSession"} || $self->id->generate;
-	$sessionId = $self->id->generate if ($sessionId !~ m/^[A-Za-z0-9\+\/=]{22}$/);
+	$sessionId = $self->id->generate unless $self->id->valid($sessionId);
 	my $noFuss = shift;
 	$self->{_var} = WebGUI::Session::Var->new($self,$sessionId, $noFuss);
 	$self->errorHandler->warn("You've disabled cache in your config file and that can cause many problems on a production site.") if ($config->get("disableCache"));
diff --git a/lib/WebGUI/Session/Id.pm b/lib/WebGUI/Session/Id.pm
index 73140db86..aecaf874f 100644
--- a/lib/WebGUI/Session/Id.pm
+++ b/lib/WebGUI/Session/Id.pm
@@ -68,8 +68,7 @@ sub generate {
   	my($s,$us)=gettimeofday();
   	my($v)=sprintf("%09d%06d%10d%06d%255s",rand(999999999),$us,$s,$$,$self->session->config->getFilename);
 	my $id = Digest::MD5::md5_base64($v);
-	$id =~ s/\+/_/g;
-	$id =~ s/\//-/g;
+	$id =~ tr{+/}{_-};
 	return $id;
 }
 
@@ -105,6 +104,19 @@ sub session {
 	return $self->{_session};
 }
 
+#-------------------------------------------------------------------
+
+=head2 valid ( $idString ) 
+
+Returns true if $idString is a valid WebGUI guid.
+
+=cut
+
+sub valid {
+	my ($self, $idString) = @_;
+	return $idString =~ m/^[A-Za-z0-9_-]{22}$/;
+}
+
 
 1;
 
diff --git a/t/Macro/GroupText.t b/t/Macro/GroupText.t
index e20bed2f0..3a601b825 100644
--- a/t/Macro/GroupText.t
+++ b/t/Macro/GroupText.t
@@ -23,7 +23,7 @@ my $session = WebGUI::Test->session;
 
 use Test::More; # increment this value for each test you create
 
-plan tests => 4 + 4;
+plan tests => 3 + 4;
 
 unless ($session->config->get('macros')->{'GroupText'}) {
 	Macro_Config::insert_macro($session, 'GroupText', 'GroupText');
diff --git a/t/Session/Id.t b/t/Session/Id.t
index 6af77e713..150bb006e 100644
--- a/t/Session/Id.t
+++ b/t/Session/Id.t
@@ -14,21 +14,64 @@ use lib "$FindBin::Bin/../lib";
 
 use WebGUI::Test;
 use WebGUI::Session;
-
-use Test::More tests => 2; # increment this value for each test you create
 use WebGUI::Utility;
 
+use Test::More;
+
+my @testSets = (
+	{
+		comment => 'too short',
+		guid => 'tooShort',
+		valid => '',
+	},
+	{
+		comment => 'too long',
+		guid => '12345678901234567890123',
+		valid => '',
+	},
+	{
+		comment => 'contains white space',
+		guid => ' 23456	890123456789012',
+		valid => '',
+	},
+	{
+		comment => 'contains illegal punctuation',
+		guid => '12#4%67*901234&678901.',
+		valid => '',
+	},
+	{
+		comment => 'MD5 style',
+		guid => '==//abcdeZYXWV01234567',
+		valid => '',
+	},
+	{
+		comment => 'GUID style',
+		guid => '--__abcdeZYXWV0123456A',
+		valid => 1,
+	},
+);
+
 my $session = WebGUI::Test->session;
 
+plan tests => scalar(@testSets) + 3;
+
 # generate
 my $generateId = $session->id->generate();
 is(length($generateId), 22, "generate() - length of 22 characters");
 my @uniqueIds;
 my $isUnique = 1;
+my $isValid = 1;
 for (1..2000) {
 	last unless $isUnique;
 	my $id = $session->id->generate();
-	$isUnique = !isIn($id,@uniqueIds);
+	$isUnique = ($isUnique ? !isIn($id,@uniqueIds) : 0);
+	$isValid = ($isValid ? $session->id->valid($id) : 0);
 	push(@uniqueIds,$id);
 }
 ok($isUnique, "generate() - unique");
+ok($isValid, "generate() - valid id generated");
+
+foreach my $testSet (@testSets) {
+	is($session->id->valid($testSet->{guid}), $testSet->{valid}, $testSet->{comment});
+}
+
diff --git a/t/Session/Scratch.t b/t/Session/Scratch.t
index 6e12e0e70..06b53cd5f 100644
--- a/t/Session/Scratch.t
+++ b/t/Session/Scratch.t
@@ -58,7 +58,7 @@ for (my $count = 1; $count <= $maxCount; $count++){
 ##Creating a new session with the previous session's Id should clone the scratch data
 my $newSession = WebGUI::Session->open(WebGUI::Test->root, WebGUI::Test->file, undef, undef, $session->getId);
 
-ok($newSession->getId eq $session->getId, "Successful session duplication");
+is($newSession->getId, $session->getId, "Successful session duplication");
 
 for (my $count = 1; $count <= $maxCount; $count++){
    is($newSession->scratch->get("dBase$count"), $count, "Passed set/get $count");