287319 - calling unused methods would break OS/2 d&d

Status: RESOLVED FIXED

(Core :: DOM: Copy & Paste and Drag & Drop, defect)

Description

[Rich Walsh]

The OS/2 version of nsDragService doesn't use the methods StartDragSession() or
EndDragSession() which are inherited from nsBaseDragService.  If some code were
to call them (in error), they would break our implementation.  This patch
overrides these two methods and turns them into no-ops to prevent this possibility.

Comment 1

[Rich Walsh]

Attachment: patch to nsDragService for OS/2

This patch should be applied to the trunk and all applicable branches.

Comment 2

[Peter Weilbacher]

This is a security measure against malitious extensions following bug 285438? Or
does it have implications that I don't see because of which I should refresh my
unofficial 1.7.6?
As a nit, perhaps you could return NS_ERROR_NOT_IMPLEMENTED instead of NS_OK?

Comment 3

[Rich Walsh]

(In reply to comment #2)
> This is a security measure against malitious extensions following bug 285438?
> Or does it have implications that I don't see because of which I should
> refresh my unofficial 1.7.6?

Not really.  This is just protection against future additions to the codebase
doing something stupid.  The two methods are declared as public, so anyone
anywhere could call them in error and bolix up OS/2 d&d.  Had they been declared
as protected, so that only subclasses of nsBaseDragService could call them, then
this wouldn't be needed.

WRT the security bug, the OS/2 version of nsDragService had _no_ problems.

> As a nit, perhaps you could return NS_ERROR_NOT_IMPLEMENTED instead of NS_OK?

Returning an error might break stupid code and give the mistaken impression that
our code was at fault.  IMHO, it's better to let whatever proceed in blissful
ignorance, generating an assertion in the debug version but otherwise doing no harm.

Comment 4

[Peter Weilbacher]

Comment on attachment 178330 [details] [diff] [review]
patch to nsDragService for OS/2

Fine, you've convinced me. I hope I am not crossing the line by marking r=me.

Comment 5

[Mike Kaply [:mkaply]]

Comment on attachment 178330 [details] [diff] [review]
patch to nsDragService for OS/2

sr=mkaply

Comment 6

[Mike Kaply [:mkaply]]

I put this on trunk. It's getting harder to get non security stuff on the
branch. If you can come up with a really good reason...

Comment 7

[Rich Walsh]

(In reply to comment #6)
> I put this on trunk. It's getting harder to get non security stuff on the
> branch. If you can come up with a really good reason...

This is simply insurance against poorly written new features.  If there are no
new features added to a branch, no insurance is needed.