686904 - Removing an invalid URL from the See Also field crashes Bugzilla

Status: RESOLVED FIXED

(Bugzilla :: WebService, defect)

Description

[Frédéric Buclin]

When webservice_bug_update_see_also.t passes

  { ids => ['public_bug'], remove => ['asdfasdfasdf'] }

to Bug.update_see_also, Bugzilla crashes with:

# Can't call method "isa" on an undefined value at Bugzilla/Bug.pm line 2892.
#  at Bugzilla/Bug.pm line 2892
#       Bugzilla::Bug::remove_see_also('Bugzilla::Bug=HASH(0x9f997a0)', 'asdfasdfasdf') called at Bugzilla/WebService/Bug.pm line 734
#       Bugzilla::WebService::Bug::update_see_also('Bugzilla::WebService::Server::JSONRPC::Bugzilla::WebService::...', 'HASH(0x9ad85d8)') called at (eval 1059) line 1
#       eval ' $procedure->{code}->($self, @params) 
# ;' called at /usr/lib/perl5/vendor_perl/5.12.2/JSON/RPC/Server.pm line 220
#       JSON::RPC::Server::_handle('Bugzilla::WebService::Server::JSONRPC::Bugzilla::WebService::...', 'HASH(0x9ad85b8)') called at Bugzilla/WebService/Server/JSONRPC.pm line 258
#       Bugzilla::WebService::Server::JSONRPC::_handle('Bugzilla::WebService::Server::JSONRPC::Bugzilla::WebService::...', 'HASH(0x9ad85b8)') called at /usr/lib/perl5/vendor_perl/5.12.2/JSON/RPC/Server.pm line 126
#       JSON::RPC::Server::handle('Bugzilla::WebService::Server::JSONRPC::Bugzilla::WebService::...') called at /var/www/html/bugzilla_qa42/jsonrpc.cgi line 41


Line 2892 in Bugzilla/Bug.pm is in remove_see_also():

  if ($removed_bug_url->isa('Bugzilla::BugUrl::Bugzilla::Local')

This means $removed_bug_url is undefined. No idea if that's expected or not.

Comment 1

[Frédéric Buclin]

Attachment: patch, v1

$removed_bug_url is undefined if the given URL wasn't in the See Also list. So we have to check for it existence first.

Comment 2

[Tiago Mello [:timello]]

Comment on attachment 560721 [details] [diff] [review]
patch, v1

Review of attachment 560721 [details] [diff] [review]:
-----------------------------------------------------------------

::: Bugzilla/Bug.pm
@@ +2889,5 @@
>      # Since we remove also the url from the referenced bug,
>      # we need to notify changes for that bug too.
>      $removed_bug_url = $removed_bug_url->[0];
> +    if ($removed_bug_url
> +        and $removed_bug_url->isa('Bugzilla::BugUrl::Bugzilla::Local')

Well, actually, I don't think we should silent ignore invalid URL when trying to remove them... we should let the user know that he is trying to remove a URL which is not valid or does not exist for that bug.

Comment 3

[Frédéric Buclin]

Comment on attachment 560721 [details] [diff] [review]
patch, v1

No, the doc for the Bug.update_see_also WebService states that "If you specify a URL that is not in the See Also field of a particular bug, it will just be silently ignored.". Throwing an error would break backward-compatibility, and it has always worked this way since Bugzilla 3.4.

Comment 4

[Tiago Mello [:timello]]

Comment on attachment 560721 [details] [diff] [review]
patch, v1

It fixes the problem.

Comment 5

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Bug.pm
Committed revision 7965.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.2/
modified Bugzilla/Bug.pm
Committed revision 7933.