Commit 9e1a1ce1 authored by Adam Simpkins's avatar Adam Simpkins Committed by Facebook Github Bot

logging: split FileHandlerFactory into two classes

Summary:
Split FileHandlerFactory into separate FileHandlerFactory and
StreamHandlerFactory classes, where FileHandlerFactory only handles logging to
files by path name, and StreamHandlerFactory only supports logging to stdout or
stderr.

The primary motivation for this is to allow logging to stdout or stderr in some
cases without allowing arbitrary files to be opened by FileHandlerFactory.
This can be achieved now by registering StreamHandlerFactory but not
FileHandlerFactory.  This makes it safer to allow controlling logging
configuration via command line flags even in setuid binaries.

Reviewed By: yfeldblum

Differential Revision: D6494226

fbshipit-source-id: a3ec371ca4266424d07dff20be18e6e13c057593
parent f805cad2
...@@ -158,6 +158,7 @@ nobase_follyinclude_HEADERS = \ ...@@ -158,6 +158,7 @@ nobase_follyinclude_HEADERS = \
experimental/LockFreeRingBuffer.h \ experimental/LockFreeRingBuffer.h \
experimental/logging/AsyncFileWriter.h \ experimental/logging/AsyncFileWriter.h \
experimental/logging/FileHandlerFactory.h \ experimental/logging/FileHandlerFactory.h \
experimental/logging/FileWriterFactory.h \
experimental/logging/GlogStyleFormatter.h \ experimental/logging/GlogStyleFormatter.h \
experimental/logging/ImmediateFileWriter.h \ experimental/logging/ImmediateFileWriter.h \
experimental/logging/Init.h \ experimental/logging/Init.h \
...@@ -181,6 +182,7 @@ nobase_follyinclude_HEADERS = \ ...@@ -181,6 +182,7 @@ nobase_follyinclude_HEADERS = \
experimental/logging/RateLimiter.h \ experimental/logging/RateLimiter.h \
experimental/logging/StandardLogHandler.h \ experimental/logging/StandardLogHandler.h \
experimental/logging/StandardLogHandlerFactory.h \ experimental/logging/StandardLogHandlerFactory.h \
experimental/logging/StreamHandlerFactory.h \
experimental/logging/xlog.h \ experimental/logging/xlog.h \
experimental/NestedCommandLineApp.h \ experimental/NestedCommandLineApp.h \
experimental/observer/detail/Core.h \ experimental/observer/detail/Core.h \
......
...@@ -15,16 +15,10 @@ ...@@ -15,16 +15,10 @@
*/ */
#include <folly/experimental/logging/FileHandlerFactory.h> #include <folly/experimental/logging/FileHandlerFactory.h>
#include <folly/Conv.h> #include <folly/experimental/logging/FileWriterFactory.h>
#include <folly/experimental/logging/AsyncFileWriter.h>
#include <folly/experimental/logging/ImmediateFileWriter.h>
#include <folly/experimental/logging/StandardLogHandler.h> #include <folly/experimental/logging/StandardLogHandler.h>
#include <folly/experimental/logging/StandardLogHandlerFactory.h> #include <folly/experimental/logging/StandardLogHandlerFactory.h>
using std::make_shared;
using std::shared_ptr;
using std::string;
namespace folly { namespace folly {
class FileHandlerFactory::WriterFactory class FileHandlerFactory::WriterFactory
...@@ -32,73 +26,27 @@ class FileHandlerFactory::WriterFactory ...@@ -32,73 +26,27 @@ class FileHandlerFactory::WriterFactory
public: public:
bool processOption(StringPiece name, StringPiece value) override { bool processOption(StringPiece name, StringPiece value) override {
if (name == "path") { if (name == "path") {
if (!stream_.empty()) {
throw std::invalid_argument(
"cannot specify both \"path\" and \"stream\" "
"parameters for FileHandlerFactory");
}
path_ = value.str(); path_ = value.str();
return true; return true;
} else if (name == "stream") {
if (!path_.empty()) {
throw std::invalid_argument(
"cannot specify both \"path\" and \"stream\" "
"parameters for FileHandlerFactory");
}
stream_ = value.str();
return true;
} else if (name == "async") {
async_ = to<bool>(value);
return true;
} else if (name == "max_buffer_size") {
auto size = to<size_t>(value);
if (size == 0) {
throw std::invalid_argument(to<string>("must be a postive number"));
}
maxBufferSize_ = size;
return true;
} else {
return false;
} }
// TODO: In the future it would be nice to support log rotation, and
// add parameters to control when the log file should be rotated.
return fileWriterFactory_.processOption(name, value);
} }
std::shared_ptr<LogWriter> createWriter() override { std::shared_ptr<LogWriter> createWriter() override {
// Get the output file to use // Get the output file to use
File outputFile; if (path_.empty()) {
if (!path_.empty()) { throw std::invalid_argument("no path specified for file handler");
outputFile = File{path_, O_WRONLY | O_APPEND | O_CREAT};
} else if (stream_ == "stderr") {
outputFile = File{STDERR_FILENO, /* ownsFd */ false};
} else if (stream_ == "stdout") {
outputFile = File{STDOUT_FILENO, /* ownsFd */ false};
} else {
throw std::invalid_argument(to<string>(
"unknown stream for FileHandlerFactory: \"",
stream_,
"\" expected one of stdout or stderr"));
}
// Determine whether we should use ImmediateFileWriter or AsyncFileWriter
if (async_) {
auto asyncWriter = make_shared<AsyncFileWriter>(std::move(outputFile));
if (maxBufferSize_.hasValue()) {
asyncWriter->setMaxBufferSize(maxBufferSize_.value());
}
return asyncWriter;
} else {
if (maxBufferSize_.hasValue()) {
throw std::invalid_argument(to<string>(
"the \"max_buffer_size\" option is only valid for async file "
"handlers"));
}
return make_shared<ImmediateFileWriter>(std::move(outputFile));
} }
return fileWriterFactory_.createWriter(
File{path_, O_WRONLY | O_APPEND | O_CREAT});
} }
std::string path_; std::string path_;
std::string stream_; FileWriterFactory fileWriterFactory_;
bool async_{true};
Optional<size_t> maxBufferSize_;
}; };
std::shared_ptr<LogHandler> FileHandlerFactory::createHandler( std::shared_ptr<LogHandler> FileHandlerFactory::createHandler(
......
...@@ -23,8 +23,13 @@ namespace folly { ...@@ -23,8 +23,13 @@ namespace folly {
* FileHandlerFactory is a LogHandlerFactory that constructs log handlers * FileHandlerFactory is a LogHandlerFactory that constructs log handlers
* that write to a file. * that write to a file.
* *
* It can construct handlers that use either ImmediateFileWriter or * Note that FileHandlerFactory allows opening and appending to arbitrary files
* AsyncFileWriter. * based on the handler options. This may make it unsafe to use
* FileHandlerFactory in some contexts: for instance, a setuid binary should
* generally avoid registering the FileHandlerFactory if they allow log
* handlers to be configured via command line parameters, since otherwise this
* may allow non-root users to append to files that they otherwise would not
* have write permissions for.
*/ */
class FileHandlerFactory : public LogHandlerFactory { class FileHandlerFactory : public LogHandlerFactory {
public: public:
......
/*
* Copyright 2004-present Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <folly/experimental/logging/FileWriterFactory.h>
#include <folly/Conv.h>
#include <folly/File.h>
#include <folly/experimental/logging/AsyncFileWriter.h>
#include <folly/experimental/logging/ImmediateFileWriter.h>
using std::make_shared;
using std::string;
namespace folly {
bool FileWriterFactory::processOption(StringPiece name, StringPiece value) {
if (name == "async") {
async_ = to<bool>(value);
return true;
} else if (name == "max_buffer_size") {
auto size = to<size_t>(value);
if (size == 0) {
throw std::invalid_argument(to<string>("must be a positive integer"));
}
maxBufferSize_ = size;
return true;
} else {
return false;
}
}
std::shared_ptr<LogWriter> FileWriterFactory::createWriter(File file) {
// Determine whether we should use ImmediateFileWriter or AsyncFileWriter
if (async_) {
auto asyncWriter = make_shared<AsyncFileWriter>(std::move(file));
if (maxBufferSize_.hasValue()) {
asyncWriter->setMaxBufferSize(maxBufferSize_.value());
}
return asyncWriter;
} else {
if (maxBufferSize_.hasValue()) {
throw std::invalid_argument(to<string>(
"the \"max_buffer_size\" option is only valid for async file "
"handlers"));
}
return make_shared<ImmediateFileWriter>(std::move(file));
}
}
} // namespace folly
/*
* Copyright 2004-present Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#pragma once
#include <folly/Optional.h>
#include <folly/Range.h>
#include <memory>
namespace folly {
class File;
class LogWriter;
/**
* A helper class for creating an AsyncFileWriter or ImmediateFileWriter based
* on log handler options settings.
*
* This is used by StreamHandlerFactory and FileHandlerFactory.
*/
class FileWriterFactory {
public:
bool processOption(StringPiece name, StringPiece value);
std::shared_ptr<LogWriter> createWriter(File file);
private:
bool async_{true};
Optional<size_t> maxBufferSize_;
};
} // namespace folly
...@@ -15,10 +15,10 @@ ...@@ -15,10 +15,10 @@
*/ */
#include <folly/experimental/logging/Init.h> #include <folly/experimental/logging/Init.h>
#include <folly/experimental/logging/FileHandlerFactory.h>
#include <folly/experimental/logging/LogConfig.h> #include <folly/experimental/logging/LogConfig.h>
#include <folly/experimental/logging/LogConfigParser.h> #include <folly/experimental/logging/LogConfigParser.h>
#include <folly/experimental/logging/LoggerDB.h> #include <folly/experimental/logging/LoggerDB.h>
#include <folly/experimental/logging/StreamHandlerFactory.h>
namespace folly { namespace folly {
...@@ -44,16 +44,24 @@ namespace folly { ...@@ -44,16 +44,24 @@ namespace folly {
* handler, the handler will be automatically forgotten by the LoggerDB code. * handler, the handler will be automatically forgotten by the LoggerDB code.
*/ */
constexpr StringPiece kDefaultLoggingConfig = constexpr StringPiece kDefaultLoggingConfig =
".=WARN:default; default=file,stream=stderr,async=false"; ".=WARN:default; default=stream,stream=stderr,async=false";
void initLogging(StringPiece configString) { void initLogging(StringPiece configString) {
// Register the FileHandlerFactory // Register the StreamHandlerFactory
// LoggerDB::get()->registerHandlerFactory(
std::make_unique<StreamHandlerFactory>());
// TODO: In the future it would be nice to build a better mechanism so that // TODO: In the future it would be nice to build a better mechanism so that
// additional LogHandlerFactory objects could be automatically registered on // additional LogHandlerFactory objects could be automatically registered on
// startup if they are linked into our current executable. // startup if they are linked into our current executable.
LoggerDB::get()->registerHandlerFactory( //
std::make_unique<FileHandlerFactory>()); // For now we register only the StreamHandlerFactory. There is a
// FileHandlerFactory, but we do not register it by default: it allows
// appending to arbitrary files based on the config settings, and we do not
// want to allow this by default for security reasons. (In the future
// maybe it would be worth enabling the FileHandlerFactory by default if we
// confirm that we are not a setuid or setgid binary. i.e., if the real
// UID+GID is the same as the effective UID+GID.)
// Parse the default log level settings before processing the input config // Parse the default log level settings before processing the input config
// string. // string.
......
...@@ -5,6 +5,7 @@ lib_LTLIBRARIES = libfollylogging.la ...@@ -5,6 +5,7 @@ lib_LTLIBRARIES = libfollylogging.la
libfollylogging_la_SOURCES = \ libfollylogging_la_SOURCES = \
AsyncFileWriter.cpp \ AsyncFileWriter.cpp \
FileHandlerFactory.cpp \ FileHandlerFactory.cpp \
FileWriterFactory.cpp \
GlogStyleFormatter.cpp \ GlogStyleFormatter.cpp \
ImmediateFileWriter.cpp \ ImmediateFileWriter.cpp \
Init.cpp \ Init.cpp \
...@@ -24,6 +25,7 @@ libfollylogging_la_SOURCES = \ ...@@ -24,6 +25,7 @@ libfollylogging_la_SOURCES = \
RateLimiter.cpp \ RateLimiter.cpp \
StandardLogHandler.cpp \ StandardLogHandler.cpp \
StandardLogHandlerFactory.cpp \ StandardLogHandlerFactory.cpp \
StreamHandlerFactory.cpp \
xlog.cpp xlog.cpp
libfollylogging_la_LIBADD = $(top_builddir)/libfolly.la libfollylogging_la_LIBADD = $(top_builddir)/libfolly.la
......
/*
* Copyright 2004-present Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <folly/experimental/logging/StreamHandlerFactory.h>
#include <folly/Conv.h>
#include <folly/experimental/logging/FileWriterFactory.h>
#include <folly/experimental/logging/StandardLogHandler.h>
#include <folly/experimental/logging/StandardLogHandlerFactory.h>
namespace folly {
class StreamHandlerFactory::WriterFactory
: public StandardLogHandlerFactory::WriterFactory {
public:
bool processOption(StringPiece name, StringPiece value) override {
if (name == "stream") {
stream_ = value.str();
return true;
}
return fileWriterFactory_.processOption(name, value);
}
std::shared_ptr<LogWriter> createWriter() override {
// Get the output file to use
File outputFile;
if (stream_.empty()) {
throw std::invalid_argument(
"no stream name specified for stream handler");
} else if (stream_ == "stderr") {
outputFile = File{STDERR_FILENO, /* ownsFd */ false};
} else if (stream_ == "stdout") {
outputFile = File{STDOUT_FILENO, /* ownsFd */ false};
} else {
throw std::invalid_argument(to<std::string>(
"unknown stream \"",
stream_,
"\": expected one of stdout or stderr"));
}
return fileWriterFactory_.createWriter(std::move(outputFile));
}
std::string stream_;
FileWriterFactory fileWriterFactory_;
};
std::shared_ptr<LogHandler> StreamHandlerFactory::createHandler(
const Options& options) {
WriterFactory writerFactory;
return StandardLogHandlerFactory::createHandler(
getType(), &writerFactory, options);
}
} // namespace folly
/*
* Copyright 2004-present Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#pragma once
#include <folly/experimental/logging/LogHandlerFactory.h>
namespace folly {
/**
* StreamHandlerFactory is a LogHandlerFactory that constructs log handlers
* that write to stdout or stderr.
*
* This is quite similar to FileHandlerFactory, but it always writes to an
* existing open file descriptor rather than opening a new file. This handler
* factory is separate from FileHandlerFactory primarily for safety reasons:
* FileHandlerFactory supports appending to arbitrary files via config
* parameters, while StreamHandlerFactory does not.
*/
class StreamHandlerFactory : public LogHandlerFactory {
public:
StringPiece getType() const override {
return "stream";
}
std::shared_ptr<LogHandler> createHandler(const Options& options) override;
private:
class WriterFactory;
};
} // namespace folly
...@@ -143,21 +143,21 @@ Example log configuration strings: ...@@ -143,21 +143,21 @@ Example log configuration strings:
therefore be discarded, even though they are enabled for one of its parent therefore be discarded, even though they are enabled for one of its parent
categories. categories.
* `ERROR:stderr, folly=INFO; stderr=file,stream=stderr` * `ERROR:stderr, folly=INFO; stderr=stream,stream=stderr`
Sets the root log category level to ERROR, and sets its handler list to Sets the root log category level to ERROR, and sets its handler list to
use the "stderr" handler. Sets the folly log level to INFO. Defines use the "stderr" handler. Sets the folly log level to INFO. Defines
a log handler named "stderr" which writes to stderr. a log handler named "stderr" which writes to stderr.
* `ERROR:default,folly=INFO:x;default=file,stream=stderr;x=file,path=/tmp/x.log` * `ERROR:x,folly=INFO:y;x=stream,stream=stderr;y=file,path=/tmp/y.log`
Defines two log handlers: "default" which writes to stderr and "x" which Defines two log handlers: "x" which writes to stderr and "y" which
writes to the file /tmp/x.log writes to the file /tmp/y.log
Sets the root log catgory level to ERROR, and configures it to use the Sets the root log catgory level to ERROR, and configures it to use the
"default" handler. Sets the log level for the "folly" category to INFO and "x" handler. Sets the log level for the "folly" category to INFO and
configures it to use the "x" handler. configures it to use the "y" handler.
* `ERROR:default:x; default=file,stream=stderr; x=file,path=/tmp/x.log` * `ERROR:default:x; default=stream,stream=stderr; x=file,path=/tmp/x.log`
Defines two log handlers: "default" which writes to stderr and "x" which Defines two log handlers: "default" which writes to stderr and "x" which
writes to the file /tmp/x.log writes to the file /tmp/x.log
...@@ -172,7 +172,7 @@ Example log configuration strings: ...@@ -172,7 +172,7 @@ Example log configuration strings:
to the empty list. Not specifying handler information at all (no ':') to the empty list. Not specifying handler information at all (no ':')
will leave any pre-existing handlers as-is. will leave any pre-existing handlers as-is.
* `;default=file,stream=stdout` * `;default=stream,stream=stdout`
Does not change any log category settings, and defines a "default" handler Does not change any log category settings, and defines a "default" handler
that writes to stdout. This format is useful to update log handler settings that writes to stdout. This format is useful to update log handler settings
...@@ -252,7 +252,7 @@ following fields: ...@@ -252,7 +252,7 @@ following fields:
} }
"handlers": { "handlers": {
"stderr": { "stderr": {
"type": "file", "type": "stream",
"options": { "options": {
"stream": "stderr", "stream": "stderr",
"async": true, "async": true,
......
...@@ -69,12 +69,12 @@ class FatalTests(unittest.TestCase): ...@@ -69,12 +69,12 @@ class FatalTests(unittest.TestCase):
subprocess.check_output([self.helper, '--crash=no']) subprocess.check_output([self.helper, '--crash=no'])
def test_async(self): def test_async(self):
handler_setings = 'default=file,stream=stderr,async=true' handler_setings = 'default=stream,stream=stderr,async=true'
err = self.run_helper('--logging=;' + handler_setings) err = self.run_helper('--logging=;' + handler_setings)
self.assertRegex(err, self.get_crash_regex()) self.assertRegex(err, self.get_crash_regex())
def test_immediate(self): def test_immediate(self):
handler_setings = 'default=file,stream=stderr,async=false' handler_setings = 'default=stream,stream=stderr,async=false'
err = self.run_helper('--logging=;' + handler_setings) err = self.run_helper('--logging=;' + handler_setings)
self.assertRegex(err, self.get_crash_regex()) self.assertRegex(err, self.get_crash_regex())
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment