Ensure that C++ exceptions do not propagate out of entry points

[eric@adblockplus.org]

Background

The plugin code is not friendly to exceptions. In most places in the code, an exception will propagate upward all the way up the stack to the entry point. At the time of creating this ticket, none of the entry points have exception handlers (try-catch block), and so such exceptions will lead to undefined behavior.

Since the C++ idiom is to use exceptions as an ordinary way to signal certain rare circumstances (memory allocation, invalid constructor calls, etc.), the absence of such handlers is a fundamental quality problem.

What to change

• Ensure that all entry points have an exception handler that will catch all thrown exceptions, that is, a catch(...) statement on the outermost block.
• Ideally, use entry point calling conventions only for entry points. This will simplify auditing the code to ensure that this change as finished.

[eric@adblockplus.org]

Entry points (as a rule) don't use C++ calling conventions, and thus tend to be marked by platform-specific annotations, such as WINAPI and STDMETHODIMP, both of whose definitions include the annotation __stdcall.

• The entry points associated with DLL packaging. They are in Plugin.cpp: DllMain, DllCanUnloadNow, DllRegisterServer, DllUnregisterServer
• BHO entry points from implementing IObjectWithSite, namely CPluginClass::SetSite.
• The browser event handler CPluginClass::Invoke.
• The COM entry points from implementing the user settings class CPluginUserSettings.
• The event handlers in WBPassthruSink.
• The COM entry points from implementing IInternetProtocolImpl in the PassthroughAPP library.

I believe this enumerates most of the entry points, though I'm not making any claim that it's all of them.

[eric@adblockplus.org]

Changeset 740 ea510d07ff97f1b50ef5389c0918276d843df391 fixes this for CPluginClass::Invoke

[sergz]

Wrap more entry points in PluginClass.cpp and add the proposal for the wrapper function. Having these functions we don't have to repeat try-catch and can improve catching for all entry points at once. As well as we don't need to comment each method.

[sergz]

​Oleksandr said:

So currently I agree with Eric. Let's just stick to try-catch blocks for now.

[eric@adblockplus.org]

More entry points.

• In CPluginClass interface `IOleCommandTarget'
  • QueryStatus()
  • Exec()

• In CPluginClass, callbacks for UI elements
  • NewStatusProc(), status bar windows
  • PaneWindowProc(), window class of status bar