Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add %byte{value} logformat code for logging or sending arbitrary bytes #236

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
af8b89b
Add %byte{value} logformat code for logging or sending arbitrary bytes
eduard-bagdasaryan Nov 9, 2023
2f72f5f
Eliminated code duplication in ProxyProtocol::IntegerToFieldType()
eduard-bagdasaryan Nov 9, 2023
44ed8a0
Moved ParseInteger() helper into a dedicated header
eduard-bagdasaryan Nov 13, 2023
44dd130
Added an std header
eduard-bagdasaryan Nov 13, 2023
edf6af0
Moved implementation details into a Parser::Impl namespace
eduard-bagdasaryan Nov 14, 2023
675b391
Eliminated code duplication in AnyP::Uri::parsePort()
eduard-bagdasaryan Nov 14, 2023
1607848
Eliminated code duplication in Ip::NfMarkConfig::Parse()
eduard-bagdasaryan Nov 14, 2023
ed4fde4
Removed trailing whitespaces
eduard-bagdasaryan Nov 14, 2023
2b55e7d
Autoformatted
eduard-bagdasaryan Nov 14, 2023
2c97d6b
Polished
eduard-bagdasaryan Nov 14, 2023
f9c8c47
fixup: Renamed Parser::Impl to Detail_
rousskov Nov 14, 2023
f2481ae
fixup: Use a more common namespace closing comment format
rousskov Nov 14, 2023
dbcafa5
fixup: Polished new names
rousskov Nov 14, 2023
29d34c9
fixup: Detail_::DecimalInteger() should not assume 0 is in range
rousskov Nov 14, 2023
a1640a2
fixup: Do not describe individual ByteCode_t values
rousskov Nov 14, 2023
c10e66b
fixup: Use a slightly more logical place for declaring LFT_BYTE
rousskov Nov 14, 2023
9f218e9
fixup: Added missing description to the new function
rousskov Nov 14, 2023
2b1f592
Revert branch AnyP::Uri changes
rousskov Nov 14, 2023
7b3b554
Revert branch Ip::NfMarkConfig changes
rousskov Nov 14, 2023
9dea5e9
Fixed branch code doing PROXY protocol TLV parsing
rousskov Nov 14, 2023
7adfe65
Cancel failed branch attempt to reduce code duplication
rousskov Nov 15, 2023
e890a64
fixup: Tightened parsing to avoid overpromising and underdeliveriing
rousskov Nov 15, 2023
2f4002d
fixup: We _need_ these limits to be constexpr
rousskov Nov 15, 2023
799f484
fixup: Slightly better names
rousskov Nov 15, 2023
af1e7c7
fixup: Added a tricky/important example of an invalid input
rousskov Nov 15, 2023
9ccf1b7
fixup: Formatted modified sources
rousskov Nov 15, 2023
5cc2517
fixup: Make our simplifying assumptions explicit
rousskov Nov 15, 2023
a985e8d
fixup: Better diagnostic for negative decimals
rousskov Nov 15, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/cf.data.pre
Original file line number Diff line number Diff line change
Expand Up @@ -4685,6 +4685,13 @@ DOC_START
Format codes:

% a literal % character

byte{value} Adds a single byte with the given value (e.g., %byte{10}
adds an ASCII LF character a.k.a. "new line" or "\n"). The value
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify: adding %byte{10} results in a genuine new line in access.log (i.e., not a couple of characters "\n"). Is this an intended/expected behavior?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this an intended/expected behavior?

Yes, it is. In fact, IIRC, adding new lines is the use case that prompted the addition of this feature. Nearly all other ASCII characters can probably be added (as those characters) without a new logformat %code.

Adding new lines is useful when, for example, access records go to a logging daemon that expects HTTP header-like record syntax.

parameter is required and must be a positive decimal integer not
exceeding 255. Zero-valued bytes (i.e. ASCII NUL characters) are
not yet supported.

sn Unique sequence number per log line entry
err_code The ID of an error response served by Squid or
a similar internal error identifier.
Expand Down
2 changes: 2 additions & 0 deletions src/format/ByteCode.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ namespace Format
typedef enum {
LFT_NONE, /* dummy */

LFT_BYTE, ///< %byte{value}

/* arbitrary string between tokens */
LFT_STRING,

Expand Down
6 changes: 6 additions & 0 deletions src/format/Format.cc
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,12 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS
out = "";
break;

case LFT_BYTE:
tmp[0] = static_cast<char>(fmt->data.byteValue);
tmp[1] = '\0';
out = tmp;
break;

case LFT_STRING:
out = fmt->data.string;
break;
Expand Down
13 changes: 13 additions & 0 deletions src/format/Token.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "format/Token.h"
#include "format/TokenTableEntry.h"
#include "globals.h"
#include "parser/Tokenizer.h"
#include "proxyp/Elements.h"
#include "sbuf/Stream.h"
#include "SquidConfig.h"
Expand Down Expand Up @@ -139,6 +140,7 @@ static TokenTableEntry TokenTable2C[] = {

/// Miscellaneous >2 byte tokens
static TokenTableEntry TokenTableMisc[] = {
TokenTableEntry("byte", LFT_BYTE),
TokenTableEntry(">eui", LFT_CLIENT_EUI),
TokenTableEntry(">qos", LFT_CLIENT_LOCAL_TOS),
TokenTableEntry("<qos", LFT_SERVER_LOCAL_TOS),
Expand Down Expand Up @@ -476,6 +478,16 @@ Format::Token::parse(const char *def, Quoting *quoting)

switch (type) {

case LFT_BYTE:
if (!data.string)
throw TextException("logformat %byte requires a parameter (e.g., %byte{10})", Here());
// TODO: Convert Format::Token::data.string to SBuf.
if (const auto v = ParseInteger<uint8_t>("logformat %byte{value}", SBuf(data.string)))
data.byteValue = v;
else
throw TextException("logformat %byte{n} does not support zero n values yet", Here());
break;

#if USE_ADAPTATION
case LFT_ADAPTATION_LAST_HEADER:
#endif
Expand Down Expand Up @@ -682,6 +694,7 @@ Format::Token::Token() : type(LFT_NONE),
data.header.element = nullptr;
data.header.separator = ',';
data.headerId = ProxyProtocol::Two::htUnknown;
data.byteValue = 0;
}

Format::Token::~Token()
Expand Down
2 changes: 2 additions & 0 deletions src/format/Token.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ class Token
char *element;
char separator;
} header;

uint8_t byteValue; // %byte{} parameter or zero
} data;
int widthMin; ///< minimum field width
int widthMax; ///< maximum field width
Expand Down
57 changes: 57 additions & 0 deletions src/parser/Tokenizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -179,5 +179,62 @@ class Tokenizer

} /* namespace Parser */

#include "sbuf/Stream.h"

/// parses a decimal integer that fits the specified Integer type
template <typename Integer>
static Integer
ParseInteger(const char *description, const SBuf &rawInput)
rousskov marked this conversation as resolved.
Show resolved Hide resolved
{
Parser::Tokenizer tok(rawInput);
if (tok.skip('0')) {
if (!tok.atEnd()) {
// e.g., 077, 0xFF, 0b101, or 0.1
throw TextException(ToSBuf("Malformed ", description,
": Expected a decimal integer without leading zeros but got '",
rawInput, "'"), Here());
}
return Integer(0);
}
// else the value might still be zero (e.g., -0)

const auto lowerLimit = std::numeric_limits<Integer>::min();
const auto upperLimit = std::numeric_limits<Integer>::max();

// check that our caller is compatible with Tokenizer::int64() use below
using ParsedInteger = int64_t;
static_assert(lowerLimit >= std::numeric_limits<ParsedInteger>::min());
static_assert(upperLimit <= std::numeric_limits<ParsedInteger>::max());

ParsedInteger rawInteger = 0;
if (!tok.int64(rawInteger, 10, true)) {
// e.g., FF
throw TextException(ToSBuf("Malformed ", description,
": Expected a decimal integer but got '",
rawInput, "'"), Here());
}

if (!tok.atEnd()) {
// e.g., 1,000, 1.0, or 1e6
throw TextException(ToSBuf("Malformed ", description,
": Trailing garbage after ", rawInteger, " in '",
rawInput, "'"), Here());
}

if (rawInteger > upperLimit) {
throw TextException(ToSBuf("Malformed ", description,
": Expected an integer value not exceeding ", upperLimit,
" but got ", rawInteger), Here());
}

if (rawInteger < lowerLimit) {
throw TextException(ToSBuf("Malformed ", description,
": Expected an integer value not below ", lowerLimit,
" but got ", rawInteger), Here());
}

return Integer(rawInteger);
}

#endif /* SQUID_PARSER_TOKENIZER_H_ */

24 changes: 1 addition & 23 deletions src/proxyp/Elements.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,29 +72,7 @@ ProxyProtocol::NameToFieldType(const SBuf &name)
ProxyProtocol::Two::FieldType
ProxyProtocol::IntegerToFieldType(const SBuf &rawInteger)
{
int64_t tlvType = 0;

Parser::Tokenizer ptok(rawInteger);
if (ptok.skip('0')) {
if (!ptok.atEnd())
throw TexcHere(ToSBuf("Invalid PROXY protocol TLV type value. ",
"Expected a decimal integer without leading zeros but got '",
rawInteger, "'")); // e.g., 077, 0xFF, or 0b101
// tlvType stays zero
} else {
Must(ptok.int64(tlvType, 10, false)); // the first character is a DIGIT
if (!ptok.atEnd())
throw TexcHere(ToSBuf("Invalid PROXY protocol TLV type value. ",
"Trailing garbage after ", tlvType, " in '",
rawInteger, "'")); // e.g., 1.0 or 5e0
}

const auto limit = std::numeric_limits<uint8_t>::max();
if (tlvType > limit)
throw TexcHere(ToSBuf("Invalid PROXY protocol TLV type value. ",
"Expected an integer less than ", limit,
" but got '", tlvType, "'"));

const auto tlvType = ParseInteger<int64_t>("proxy protocol TLV type value", rawInteger);
return Two::FieldType(tlvType);
}

Expand Down
Loading