[GitHub] [logging-log4net] tschettler commented on pull request #73: adds ActivatorProvider to allow custom type activation (Resolves LOG4NET-565)

[GitBox <gi...@apache.org>]

tschettler commented on pull request #73:
URL: https://github.com/apache/logging-log4net/pull/73#issuecomment-951019204


   Thank you for the feedback @fluffynuts!
   
   I understand your security concerns and completely agree with you. My initial commit was just to provide a standard way to create instances, without regard to whether they _needed_ to be created using a custom activator. I think the changes could be pared down based on your feedback and additional changes could be made to circumvent the security concerns you have outlined. Here are the following two areas that I think would cover most use cases:
   
     1. **A DI-enabled Appender.** 
     One scenario here would be when someone would like to use a managed ORM to log to a database. An existing DI container manages the scope and creation of these dependencies, possibly because all data access for an application needs to be configured and managed in a specific manner. Enabling this would just require the change within `XmlHierarchyConfigurator`.
     2. **A DI-enabled Pattern Converter.**
     Consider wanting to use a pattern converter to log items from the HttpContext in ASP.NET Core. A standard method to retrieve HttpContext information is to inject `IHttpContextAccessor`. `ConverterInfo` only provides the converter type and there's no way to intercept the creation of the pattern converter to provide this dependency. Enabling this would just require the change within `PatternParser`.
   
   Along with limiting the scope of changes to just be for these two classes, the `IActivator` implementation type could be registered with the log4net config and `ActivatorProvider` could perform a validation check against this type to ensure it matches. Once the instance is set, it could also protect the instance from being changed to something else. 
   
   If you would allow me, I can build out the changes as specified. With consideration to reducing the changes to account for just these scenarios, implementing some additional logic for validating the activator instance type, and locking down from further changes, what remaining security concerns would you have?
   
   Thanks!
   
   Travis


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org