Should this API sit in its own package? On Jul 22, 2017 11:41, "Mikael Ståldal" <mi...@apache.org> wrote:
> I'll give it a shot: > > https://issues.apache.org/jira/browse/LOG4J2-1986 > > > On 2017-07-22 20:28, Ralph Goers wrote: > >> Yes, that is a good idea. Probably XML and YAML as well. >> >> Ralph >> >> On Jul 22, 2017, at 11:21 AM, Mikael Ståldal <mi...@apache.org> wrote: >>> >>> But here we have a concrete use case: a third-party tool, Lilith, needs >>> to parse log events from our JsonLayout. (Our SocketServer in log4j-server >>> have the same need, and it uses this class.) >>> >>> Should we provide a public API in log4j-core for doing so, which Lilith, >>> our SocketServer, and possibly others (Chainsaw maybe?) can use? Maybe such >>> public API should be a facade around this which hides the fact that we use >>> Jackson. >>> >>> >>> On 2017-07-22 19:04, Gary Gregory wrote: >>> >>>> Hi All, >>>> Just like anything in log4j-core, we try not break binary compatibility >>>> when we can but we do not bend over backwards like we do for log4j-api. >>>> This class is public probably because Jackson requires it to be, >>>> otherwise >>>> I would have made it package private. The fact that we use Jackson for >>>> JSON >>>> IO is an implementation detail IMO, not an extension point. If anyone >>>> wants >>>> to hard-code a reference to log4j-core's guts then I think its fair to >>>> day >>>> that this comes with caveats. >>>> Gary >>>> On Sat, Jul 22, 2017 at 7:27 AM, Jörn Huxhorn <jhuxh...@googlemail.com> >>>> wrote: >>>> >>>>> Well… it says >>>>> >>>>> /** >>>>> * A Jackson JSON {@link ObjectMapper} initialized for Log4j. >>>>> * <p> >>>>> * <em>Consider this class private.</em> >>>>> * </p> >>>>> */ >>>>> >>>>> so I wanted to holler about how to continue. >>>>> >>>>> I’ll give it a shot. >>>>> >>>>> >>>>> On 22. July 2017 at 15:06:28, Mikael Ståldal (mi...@apache.org) wrote: >>>>> >>>>>> Would it work to use >>>>>> org.apache.logging.log4j.core.jackson.Log4jJsonObjectMapper, which is >>>>>> public? >>>>>> >>>>>> See here how it is used: >>>>>> >>>>>> https://github.com/apache/logging-log4j-tools/blob/ >>>>>> >>>>> master/log4j-server/src/main/java/org/apache/logging/log4j/server/ >>>>> JsonInputStreamLogEventBridge.java >>>>> >>>>>> >>>>>> >>>>>> On 2017-07-22 13:47, Jörn Huxhorn wrote: >>>>>> >>>>>>> It seems like I don’t get access to the log4j2 ObjectMapper or >>>>>>> Jackson >>>>>>> >>>>>> Module used for >>>>> >>>>>> JSON serialization, i.e. I can’t do something like this: >>>>>> >>>>>>> new JacksonFactory.JSON(encodeThreadContextAsList, >>>>>>> includeStacktrace, >>>>>>> >>>>>> false) >>>>> >>>>>> >>>>>>> It would make sense to reuse your ObjectMapper/Module in my code >>>>>>> >>>>>> instead of reimplementing >>>>> >>>>>> them to prevent future compatibility issues down the road. >>>>>> >>>>>>> >>>>>>> >>>>>>> On 19. July 2017 at 23:04:26, Jörn Huxhorn (jhuxh...@googlemail.com) >>>>>>> >>>>>> wrote: >>>>> >>>>>> I’ll give this a shot over the next view days. >>>>>>>> >>>>>>>> Thanks! >>>>>>>> >>>>>>>> >>>>>>>> On 19. July 2017 at 21:33:49, Mikael Ståldal (mi...@apache.org) >>>>>>>> >>>>>>> wrote: >>>>> >>>>>> JsonLayout (and XmlLayout and YamlLayout) now supports 0-byte >>>>>>>>> termination of log events. Will be part of the upcoming 2.9 >>>>>>>>> release. >>>>>>>>> >>>>>>>>> >>>>>>>>> On 2017-07-18 11:48, Jörn Huxhorn wrote: >>>>>>>>> >>>>>>>>>> A general event header and footer would probably be overkill. >>>>>>>>>> >>>>>>>>>> I’m using the following interface in Lilith: >>>>>>>>>> >>>>>>>>>> public interface WriteByteStrategy { >>>>>>>>>> void writeBytes(DataOutputStream dataOutputStream, byte[] bytes) >>>>>>>>>> throws IOException; >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> One implementation writes an int with the length of bytes before >>>>>>>>>> >>>>>>>>> “bytes", the other >>>>> >>>>>> implementation writes a 0-byte after “bytes". >>>>>>>>> >>>>>>>>>> >>>>>>>>>> The problem with that approach is that it requires the >>>>>>>>>> intermediate >>>>>>>>>> >>>>>>>>> “bytes” array >>>>> >>>>>> and >>>>>>>> >>>>>>>>> is thus probably incompatible with your zero garbage approach. >>>>>>>>> >>>>>>>>>> >>>>>>>>>> I didn’t try end-of-line (\r\n) mainly because I first wanted to >>>>>>>>>> >>>>>>>>> discuss this with >>>>> >>>>>> you. >>>>>>>> >>>>>>>>> I would also have to implement a new receiver type whereas I could >>>>>>>>> >>>>>>>> just reuse an existing >>>>> >>>>>> one in case of 0-byte termination. >>>>>>>>> >>>>>>>>>> >>>>>>>>>> I’d prefer using 0-byte termination since it would be less fragile >>>>>>>>>> >>>>>>>>> than end-of-line. >>>>> >>>>>> The end-of-line approach would break if compact=“false” was set >>>>>>>>> >>>>>>>> erroneously while >>>>> >>>>>> you’d still have the option of using either compact=“true” or >>>>>>>>> >>>>>>>> compact=“false” in >>>>> >>>>>> case >>>>>> >>>>>>> of 0-byte termination. 0-byte termination is also slightly more >>>>>>>>> >>>>>>>> efficient while >>>>> >>>>>> reading >>>>>> >>>>>>> since the splitting can operate on bytes instead of chars or Strings >>>>>>>>> >>>>>>>> - another reason >>>>> >>>>>> why I didn’t just implement it and instead wanted to discuss this >>>>>>>>> >>>>>>>> with you first. >>>>> >>>>>> >>>>>>>>>> Joern >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 18. July 2017 at 03:26:32, Ralph Goers ( >>>>>>>>>> >>>>>>>>> ralph.go...@dslextreme.com) wrote: >>>>> >>>>>> Right now that is all handled by the specific layouts. For >>>>>>>>>>> >>>>>>>>>> example, by default the >>>>> >>>>>> RFC5424Layout >>>>>>>> >>>>>>>>> doesn’t append newlines so when writing to a file they will all be >>>>>>>>>>> >>>>>>>>>> on the same “line”, >>>>> >>>>>> but >>>>>>>>> >>>>>>>>>> it has an option to append one if needed. Doing the same would be >>>>>>>>>>> >>>>>>>>>> another option for >>>>> >>>>>> the >>>>>>>> >>>>>>>>> JSONLayout. >>>>>>>>>>> >>>>>>>>>>> I’d be ok with event header and footers but ONLY if they have >>>>>>>>>>> zero >>>>>>>>>>> >>>>>>>>>> overhead when not >>>>> >>>>>> present. >>>>>>>> >>>>>>>>> IOW, when the Layout is initialized it would have to wire in the >>>>>>>>>>> >>>>>>>>>> appropriate encode >>>>> >>>>>> method. >>>>>>>>> >>>>>>>>>> >>>>>>>>>>> Ralph >>>>>>>>>>> >>>>>>>>>>> On Jul 17, 2017, at 3:06 PM, Gary Gregory wrote: >>>>>>>>>>>> >>>>>>>>>>>> Do we want a general event header and footer then? >>>>>>>>>>>> >>>>>>>>>>>> Gary >>>>>>>>>>>> >>>>>>>>>>>> On Jul 17, 2017 14:43, "Ralph Goers" wrote: >>>>>>>>>>>> >>>>>>>>>>>> No. A Footer is only used at end of file. He needs to know how >>>>>>>>>>>>> >>>>>>>>>>>> long each >>>>> >>>>>> event is or when it is the start of a new event. >>>>>>>>>>>>> >>>>>>>>>>>>> Ralph >>>>>>>>>>>>> >>>>>>>>>>>>> On Jul 17, 2017, at 12:32 PM, Gary Gregory >>>>>>>>>>>>>> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> Can't you use a footer for any terminator you wish? >>>>>>>>>>>>>> >>>>>>>>>>>>>> Gary >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Mon, Jul 17, 2017 at 12:13 PM, Mikael Ståldal >>>>>>>>>>>>>> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> Hi. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> (Moving this discussion to logging dev mailing list.) >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Have you tried to use: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Then each log event will be terminated by end-of-line (\r\n). >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> I think it would be easy to implement 0-byte terminated log >>>>>>>>>>>>>>> >>>>>>>>>>>>>> events in >>>>> >>>>>> JsonLayout, and that would make sense since we have >>>>>>>>>>>>>>> >>>>>>>>>>>>>> implemented support >>>>> >>>>>> for >>>>>>>>>>>>> >>>>>>>>>>>>>> that in GelfLayout. Created a JIRA issue for it: >>>>>>>>>>>>>>> https://issues.apache.org/jira/browse/LOG4J2-1981 >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> As for other tools, the only receivers for Log4j 2 >>>>>>>>>>>>>>> >>>>>>>>>>>>>> SerializedLayout we >>>>> >>>>>> know are Log4j's own SocketServer and Lilith. Chainsaw >>>>>>>>>>>>>>> >>>>>>>>>>>>>> currently only >>>>> >>>>>> support Log4j 1 SerializedLayout. Log4j's own SocketServer >>>>>>>>>>>>>>> >>>>>>>>>>>>>> support >>>>> >>>>>> JsonLayout and XmlLayout as well, and we are changing it to use >>>>>>>>>>>>>>> >>>>>>>>>>>>>> JsonLayout >>>>>>>>>>>>> >>>>>>>>>>>>>> by default. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On 2017-07-17 15:01, Joern Huxhorn wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Hi Mikael, >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I've just taken a look at the JsonLayout and see no way to >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> parse its >>>>> >>>>>> output event by event. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> It's semi-fine for files because then it will result in a >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> well-formed >>>>> >>>>>> array at the root level that can be read in one big swoop. It >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> isn't >>>>> >>>>>> really ideal for that use case either since it means that the >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> whole >>>>> >>>>>> array containing all events needs to be read into memory in >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> one piece, >>>>> >>>>>> causing OOM in case of huge files. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> But in Lilith, I really need to process events one by one. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> This is >>>>> >>>>>> absolutely crucial. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> The only way to achieve this would essentially require me to >>>>>>>>>>>>>>>> re-implement a JSON parser that can cope with data like >>>>>>>>>>>>>>>> this. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I implemented a similar kludge to support reading of log4j >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> xml files >>>>> >>>>>> (see >>>>>>>>>>>>>>>> https://github.com/huxi/lilith/blob/master/log4j/log4j-xml/ >>>>>>>>>>>>>>>> src/main/java/de/huxhorn/lilith/log4j/xml/ >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Log4jImportCallable.java >>>>> >>>>>> ) but this was way easier since i could simply search for >>>>>>>>>>>>>>>> "" to find the position at which I could split the stream >>>>>>>>>>>>>>>> of events into a valid XML document for every single event . >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> This was >>>>> >>>>>> still kind of nasty but doable, especially since processing an >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> "offline" >>>>>>>>>>>>> >>>>>>>>>>>>>> XML file doesn't have the same performance restriction as >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> handling of >>>>> >>>>>> live events. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I wouldn't have the luxury of such a unique split signal in >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> JSON, >>>>> >>>>>> though. >>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I would need to detect the "}" at the level of the root >>>>>>>>>>>>>>>> array >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> which >>>>> >>>>>> would not simply involve counting of opening and closing >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> brackets but >>>>> >>>>>> also ignoring of brackets inside of strings. This would boil >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> down to >>>>> >>>>>> implementing a custom JSON reader and I won't be doing this. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> This isn't just me being lazy either. Such an implementation >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> wouldn't >>>>> >>>>>> be >>>>>>>>>>>>> >>>>>>>>>>>>>> resource (CPU/memory) friendly anyway. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> In my own JSON receiver using my own JSON format there are >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> two ways to >>>>> >>>>>> send those events: >>>>>>>>>>>>>>>> - write an int containing the amount of bytes representing >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> the event, >>>>> >>>>>> then the bytes of the event. This type of events also supports >>>>>>>>>>>>>>>> compression. >>>>>>>>>>>>>>>> - write the bytes of the event, followed by a 0-byte. This >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> works fine >>>>> >>>>>> and JavaScript/ActionScript is able to produce events like >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> that while >>>>> >>>>>> they are unable (or were unable 7 years ago) to count the >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> bytes on >>>>> >>>>>> their >>>>>>>>>>>>> >>>>>>>>>>>>>> own. This type of events only supports plain text like JSON >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> or XML and >>>>> >>>>>> no compression since compressed events could contain the >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> 0-byte while >>>>> >>>>>> XML and JSON both won't allow it. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Both are essentially "binary" formats because of either the >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> raw ints or >>>>> >>>>>> the 0-bytes. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I'm not sure why you deprecate SerializedLayout since the >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> security >>>>> >>>>>> issues arise only while deserializing the events, not while >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> serializing >>>>> >>>>>> them. Is this just meant to educate the user about the issue? >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I fixed the remote code execution exploit by implementing >>>>>>>>>>>>>>>> https://github.com/huxi/lilith/blob/master/lilith-engine/ >>>>>>>>>>>>>>>> src/main/java/de/huxhorn/lilith/engine/impl/eventproduc >>>>>>>>>>>>>>>> er/WhitelistObjectInputStream.java >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> This still doesn't fix DoS scenarios (endless loops or OOM >>>>>>>>>>>>>>>> in >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> case of >>>>> >>>>>> malicious data) but is close enough for me, mainly because >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> fixing the >>>>> >>>>>> DoS issues would really have to be handled by Java, not by >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> user code. >>>>> >>>>>> Manually re-implementing deserialization would likely be >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> fragile and >>>>> >>>>>> error prone, possibly even introducing other security issues >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> on its >>>>> >>>>>> own. >>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I'd be interested how other tools are tackling this issue. >>>>>>>>>>>>>>>> Is >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Chainsaw >>>>> >>>>>> officially dead or are they implementing receivers for event >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> streams >>>>> >>>>>> like this? >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I'm totally fine with implementing a new receiver as a >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> replacement for >>>>> >>>>>> log4j2 SerializedLayout (even though I won't be able to get >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> rid of the >>>>> >>>>>> Serializable receiver anyway) but the current JsonLayout just >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> isn't up >>>>> >>>>>> to the job at the moment. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Feel free to share this mail with your fellow developers. >>>>>>>>>>>>>>>> I'm >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> open for >>>>> >>>>>> suggestions. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I hope this doesn't come across in a harsh way. It isn't >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> meant to be. >>>>> >>>>>> >>>>>>>>>>>>>>>> Cheers, >>>>>>>>>>>>>>>> Jörn. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>> >>>>> >>> >>> >> >> >
