Closed Bug 793826 Opened 8 years ago Closed 8 years ago

Prevent private web service methods from being called

Categories

(Bugzilla :: WebService, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 4.2

People

(Reporter: koosha.khajeh, Assigned: koosha.khajeh)

Details

Attachments

(1 file)

Attached patch patch - v1Splinter Review
Usually, we use methods (subroutines) that start with '_' for internal purposes only and they are somehow private. But, we don't check if the user is trying to call one of these private methods. It is not reasonable to be able to call these methods as a client. In addition, they might cause some security implications. It would be better to prevent these methods from being called.
Attachment #664188 - Flags: review?(LpSolit)
It should not be possible to call a private method. If we can, that's a bug.
Severity: enhancement → normal
Target Milestone: Bugzilla 4.4 → Bugzilla 4.2
It is possible to do so as I just verified it. 

One way to enforce would be add code to each private method to check if caller is the current $self otherwise die. 

sub _private_method_foo {
    my ($self) = @_;
    ThrowCodeError("private_method_called")
        unless (caller)[0]->isa( ref($self) );
    return "foo";
}

Or make each private method an anonymous code ref and call it that way such as:

my $private_method_foo = sub {
    return "foo";
};

sub public_method_foo {
    my ($self) = @_;
    return $self->$private_method_foo();
}

Of course Koosha's fix works as long as we use the same private method naming convention throughout which so far it looks like we do. So that would be the quickest fix with the least amount of code change for now.

dkl
Attachment #664188 - Flags: review?(dkl)
Comment on attachment 664188 [details] [diff] [review]
patch - v1

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

Works as described and r=dkl unless LpSolit wants to make major changes across all webservice code and do it a different/better way.
Attachment #664188 - Flags: review?(dkl) → review+
Attachment #664188 - Flags: review?(LpSolit)
(In reply to David Lawrence [:dkl] from comment #3)
> Works as described and r=dkl unless LpSolit wants to make major changes
> across all webservice code and do it a different/better way.

No, I'm fine with the way it's done in this patch.
Flags: approval4.4+
Flags: approval4.2+
Flags: approval+
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/WebService/Server.pm
Committed revision 8422.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.4/
modified Bugzilla/WebService/Server.pm
Committed revision 8416.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified Bugzilla/WebService/Server.pm
Committed revision 8149.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.