[Rt-commit] rt branch, 4.0/check-FromURI-return-value, created. rt-4.0.9-12-g6d954a4

[Thomas Sibley]

The branch, 4.0/check-FromURI-return-value has been created
        at  6d954a45e3fd0197028a6824db6b271f9b9c3d0a (commit)

- Log -----------------------------------------------------------------
commit 4f107820e8d85490d5e140a72fe3cef5f3711469
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Thu Jan 24 10:59:09 2013 -0800

    Check the return value of RT::URI->FromURI instead of ->Resolver
    
    FromURI always sets a resolver, even on failure.  Error checking which
    only looks at ->Resolver is broken and checking ->Scheme on top of that
    is just patching over the bug and relying on the internals of
    RT::URI::base.  FromURI returns a useful success or failure value; just
    use that.

diff --git a/lib/RT/Article.pm b/lib/RT/Article.pm
index 589a6b8..ec1ae3c 100644
--- a/lib/RT/Article.pm
+++ b/lib/RT/Article.pm
@@ -399,9 +399,8 @@ sub AddLink {
 
     # Check that we're actually getting a valid URI
     my $uri_obj = RT::URI->new( $self->CurrentUser );
-    $uri_obj->FromURI( $args{'Target'}||$args{'Base'} );
-    unless ( $uri_obj->Resolver && $uri_obj->Scheme ) {
-        my $msg = $self->loc( "Couldn't resolve '[_1]' into a Link.", $args{'Target'} );
+    unless ( $uri_obj->FromURI( $args{'Target'}||$args{'Base'} )) {
+        my $msg = $self->loc( "Couldn't resolve '[_1]' into a Link.", $args{'Target'} || $args{'Base'} );
         $RT::Logger->warning( $msg );
         return( 0, $msg );
     }
diff --git a/lib/RT/Link.pm b/lib/RT/Link.pm
index 7f62e4e..74217ee 100644
--- a/lib/RT/Link.pm
+++ b/lib/RT/Link.pm
@@ -96,33 +96,17 @@ sub Create {
                  @_ );
 
     my $base = RT::URI->new( $self->CurrentUser );
-    $base->FromURI( $args{'Base'} );
-
-    unless ( $base->Resolver && $base->Scheme ) {
-	my $msg = $self->loc("Couldn't resolve base '[_1]' into a URI.", 
-			     $args{'Base'});
+    unless ($base->FromURI( $args{'Base'} )) {
+        my $msg = $self->loc("Couldn't resolve base '[_1]' into a URI.", $args{'Base'});
         $RT::Logger->warning( "$self $msg" );
-
-	if (wantarray) {
-	    return(undef, $msg);
-	} else {
-	    return (undef);
-	}
+        return wantarray ? (undef, $msg) : undef;
     }
 
     my $target = RT::URI->new( $self->CurrentUser );
-    $target->FromURI( $args{'Target'} );
-
-    unless ( $target->Resolver ) {
-	my $msg = $self->loc("Couldn't resolve target '[_1]' into a URI.", 
-			     $args{'Target'});
+    unless ($target->FromURI( $args{'Target'} )) {
+        my $msg = $self->loc("Couldn't resolve target '[_1]' into a URI.", $args{'Target'});
         $RT::Logger->warning( "$self $msg" );
-
-	if (wantarray) {
-	    return(undef, $msg);
-	} else {
-	    return (undef);
-	}
+        return wantarray ? (undef, $msg) : undef;
     }
 
     my $base_id   = 0;
@@ -186,15 +170,12 @@ sub LoadByParams {
                  @_ );
 
     my $base = RT::URI->new($self->CurrentUser);
-    $base->FromURI( $args{'Base'} );
+    $base->FromURI( $args{'Base'} )
+        or return (0, $self->loc("Couldn't parse Base URI: [_1]", $args{Base}));
 
     my $target = RT::URI->new($self->CurrentUser);
-    $target->FromURI( $args{'Target'} );
-    
-    unless ($base->Resolver && $target->Resolver) {
-        return ( 0, $self->loc("Couldn't load link") );
-    }
-
+    $target->FromURI( $args{'Target'} )
+        or return (0, $self->loc("Couldn't parse Target URI: [_1]", $args{Target}));
 
     my ( $id, $msg ) = $self->LoadByCols( Base   => $base->URI,
                                           Type   => $args{'Type'},
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index 48d1442..eab2846 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -2576,9 +2576,7 @@ sub __GetTicketFromURI {
     # If the other URI is an RT::Ticket, we want to make sure the user
     # can modify it too...
     my $uri_obj = RT::URI->new( $self->CurrentUser );
-    $uri_obj->FromURI( $args{'URI'} );
-
-    unless ( $uri_obj->Resolver && $uri_obj->Scheme ) {
+    unless ($uri_obj->FromURI( $args{'URI'} )) {
         my $msg = $self->loc( "Couldn't resolve '[_1]' into a URI.", $args{'URI'} );
         $RT::Logger->warning( $msg );
         return( 0, $msg );

commit 6d954a45e3fd0197028a6824db6b271f9b9c3d0a
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Thu Jan 24 11:01:27 2013 -0800

    Return a (status, message) tuple on success from RT::Link->LoadByParams
    
    Previously only returned a tuple on failure, which is less than useful.

diff --git a/lib/RT/Link.pm b/lib/RT/Link.pm
index 74217ee..7a27747 100644
--- a/lib/RT/Link.pm
+++ b/lib/RT/Link.pm
@@ -182,7 +182,9 @@ sub LoadByParams {
                                           Target => $target->URI );
 
     unless ($id) {
-        return ( 0, $self->loc("Couldn't load link") );
+        return ( 0, $self->loc("Couldn't load link: [_1]", $msg) );
+    } else {
+        return ($id, $msg);
     }
 }
 

-----------------------------------------------------------------------