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.

Sign up for GitHub

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

Jump to bottom

issue #2228: refine config directive token parse; #4042

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

issue #2228: refine config directive token parse; #4042

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion trunk/conf/edge.token.traverse.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# @see https://ossrs.net/lts/zh-cn/docs/v4/doc/drm
# @see full.conf for detail config.

listen 1935
listen 1935;
max_connections 1000;
daemon off;
srs_log_tank console;
Expand Down
2 changes: 1 addition & 1 deletion trunk/conf/full.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2665,7 +2665,7 @@ vhost stream.transcode.srs.com {
#
# @see https://github.com/ossrs/srs/issues/1399
#############################################################################################
include include.vhost.conf;
include ./conf/include.vhost.conf;

#############################################################################################
# The origin cluster section
Expand Down
2 changes: 1 addition & 1 deletion trunk/conf/http.heartbeat.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# the config for srs http heartbeat, report its info to api-server
# @see full.conf for detail config.

listen 1935
listen 1935;
max_connections 1000;
daemon off;
srs_log_tank console;
Expand Down
40 changes: 40 additions & 0 deletions trunk/scripts/verify_confs.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#!/bin/bash

TRUNK_DIR=$(dirname $(realpath -q $0))/..

pushd $TRUNK_DIR > /dev/null

SRS_EXE=$(pwd)/objs/srs

if [ ! -f ${SRS_EXE} ]; then
echo "${SRS_EXE} not exist"
exit -1
fi

if [ ! -x ${SRS_EXE} ]; then
echo "${SRS_EXE} not executable"
exit -2
fi

for f in conf/*.conf
do
if [ -f $f ]; then
# skip below conf
if [[ $f == "conf/full.conf" ||
$f == "conf/hls.edge.conf" ||
$f == "conf/nginx.proxy.conf" ||
$f == "conf/include.vhost.conf" ]]; then
continue
fi

${SRS_EXE} -t -c $f
RET=$?
if [ $RET -ne 0 ]; then
echo "please check $f"
popd > /dev/null
exit $RET
fi
fi
done

popd > /dev/null
18 changes: 14 additions & 4 deletions trunk/src/app/srs_app_config.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1202,9 +1202,15 @@ srs_error_t SrsConfDirective::read_token(SrsConfigBuffer* buffer, vector<string>

char ch = *buffer->pos++;

if (ch == SRS_LF) {
buffer->line++;
if (ch == SRS_LF || ch == SRS_CR) {
if (ch == SRS_LF) {
buffer->line++;
}

sharp_comment = false;
if (args.size() > 0) {
return srs_error_new(ERROR_SYSTEM_CONFIG_INVALID, "line %d: unexpected end of line to parse token %s", buffer->line - 1, args[0].c_str());
}
}

if (sharp_comment) {
Expand Down Expand Up @@ -1305,7 +1311,7 @@ srs_error_t SrsConfDirective::read_token(SrsConfigBuffer* buffer, vector<string>
args.push_back(word_str);
}
srs_freepa(aword);

if (ch == ';') {
state = SrsDirectiveStateEntire;
return err;
Expand All @@ -1314,6 +1320,10 @@ srs_error_t SrsConfDirective::read_token(SrsConfigBuffer* buffer, vector<string>
state = SrsDirectiveStateBlockStart;
return err;
}

if ((ch == SRS_LF || ch == SRS_CR) && args.size() > 0) {
return srs_error_new(ERROR_SYSTEM_CONFIG_INVALID, "line %d: unexpected end of line to parse token %s", buffer->line - 1, args[0].c_str());
}
}
}
}
Expand Down Expand Up @@ -2378,7 +2388,7 @@ srs_error_t SrsConfig::check_normal_config()
string n = conf->at(i)->name;
if (n != "enabled" && n != "listen" && n != "maxbw"
&& n != "mss" && n != "latency" && n != "recvlatency"
&& n != "peerlatency" && n != "connect_timeout"
&& n != "peerlatency" && n != "connect_timeout" && n != "peer_idle_timeout"
&& n != "sendbuf" && n != "recvbuf" && n != "payloadsize"
&& n != "default_app" && n != "sei_filter" && n != "mix_correct"
&& n != "tlpktdrop" && n != "tsbpdmode" && n != "passphrase" && n != "pbkeylen") {
Expand Down
2 changes: 1 addition & 1 deletion trunk/src/utest/srs_utest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ VOID TEST(SampleTest, ContextTest)
cache[0] = cid;
}

MockProtectedBuffer::MockProtectedBuffer() : size_(0), data_(NULL), raw_memory_(NULL)
MockProtectedBuffer::MockProtectedBuffer() : raw_memory_(NULL), size_(0), data_(NULL)
Copy link
Member

Choose a reason for hiding this comment

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

Nothing changed?

Copy link
Contributor Author

@suzp1984 suzp1984 Apr 24, 2024 •
edited

Choose a reason for hiding this comment

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

Yes, just to fix a compiler warning: the init order should be the declare orders in the header file.

srs/trunk/src/utest/srs_utest.hpp

Lines 112 to 117 in 37f0faa

private:
char* raw_memory_;
public:
int size_;
// Should use this as data.
char* data_;

{
}

Expand Down
142 changes: 115 additions & 27 deletions trunk/src/utest/srs_utest_config.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,16 +400,31 @@ VOID TEST(ConfigDirectiveTest, ParseArgsSpace)
EXPECT_EQ(0, (int) conf.directives.size());
}
}

if (true) {
vector <string> usecases;
usecases.push_back("include\rtest;");
usecases.push_back("include\ntest;");
usecases.push_back("include \r \n \r\n \n\rtest;");

for (int i = 0; i < (int)usecases.size(); i++) {
string usecase = usecases.at(i);

MockSrsConfigBuffer buf(usecase);
SrsConfDirective conf;
HELPER_ASSERT_FAILED(conf.parse(&buf));
EXPECT_EQ(0, (int) conf.name.length());
EXPECT_EQ(0, (int) conf.args.size());
EXPECT_EQ(0, (int) conf.directives.size());
}
}

if (true) {
vector <string> usecases;
usecases.push_back("include test;");
usecases.push_back("include test;");
usecases.push_back("include test;");
usecases.push_back("include test;");;
usecases.push_back("include\rtest;");
usecases.push_back("include\ntest;");
usecases.push_back("include \r \n \r\n \n\rtest;");

MockSrsConfig config;
config.mock_include("test", "listen 1935;");
Expand All @@ -433,6 +448,102 @@ VOID TEST(ConfigDirectiveTest, ParseArgsSpace)
}
}

VOID TEST(ConfigDirectiveTest, ParseInvalidEndOfLine)
{
srs_error_t err;

if (true) {
MockSrsConfigBuffer buf("dir0 \narg0;dir1 arg1;");
SrsConfDirective conf;
HELPER_ASSERT_FAILED(conf.parse(&buf));
EXPECT_EQ(0, (int) conf.name.length());
EXPECT_EQ(0, (int) conf.args.size());
EXPECT_EQ(0, (int) conf.directives.size());
}

if (true) {
MockSrsConfigBuffer buf("dir0\n arg0;dir1 arg1;");
SrsConfDirective conf;
HELPER_ASSERT_FAILED(conf.parse(&buf));
EXPECT_EQ(0, (int) conf.name.length());
EXPECT_EQ(0, (int) conf.args.size());
EXPECT_EQ(0, (int) conf.directives.size());
}

if (true) {
MockSrsConfigBuffer buf("dir0 arg0\n;dir1 arg1;");
SrsConfDirective conf;
HELPER_ASSERT_FAILED(conf.parse(&buf));
EXPECT_EQ(0, (int) conf.name.length());
EXPECT_EQ(0, (int) conf.args.size());
EXPECT_EQ(0, (int) conf.directives.size());
}

if (true) {
MockSrsConfigBuffer buf("dir0 \rarg0;dir1 arg1;");
SrsConfDirective conf;
HELPER_ASSERT_FAILED(conf.parse(&buf));
EXPECT_EQ(0, (int) conf.name.length());
EXPECT_EQ(0, (int) conf.args.size());
EXPECT_EQ(0, (int) conf.directives.size());
}

if (true) {
MockSrsConfigBuffer buf("dir0 arg0\r;dir1 arg1;");
SrsConfDirective conf;
HELPER_ASSERT_FAILED(conf.parse(&buf));
EXPECT_EQ(0, (int) conf.name.length());
EXPECT_EQ(0, (int) conf.args.size());
EXPECT_EQ(0, (int) conf.directives.size());
}

if (true) {
MockSrsConfigBuffer buf("dir0 \n { dir1 arg1; }");
SrsConfDirective conf;
HELPER_ASSERT_FAILED(conf.parse(&buf));
EXPECT_EQ(0, (int) conf.name.length());
EXPECT_EQ(0, (int) conf.args.size());
EXPECT_EQ(0, (int) conf.directives.size());
}


if (true) {
MockSrsConfigBuffer buf("dir0 arg0;dir1\n arg1;");
SrsConfDirective conf;
HELPER_ASSERT_FAILED(conf.parse(&buf));
EXPECT_EQ(0, (int) conf.name.length());
EXPECT_EQ(0, (int) conf.args.size());
EXPECT_EQ(1, (int) conf.directives.size());

SrsConfDirective& dir0 = *conf.directives.at(0);
EXPECT_STREQ("dir0", dir0.name.c_str());
EXPECT_EQ(1, (int)dir0.args.size());
EXPECT_STREQ("arg0", dir0.arg0().c_str());
EXPECT_EQ(0, (int)dir0.directives.size());
}

if (true) {
MockSrsConfigBuffer buf("dir0 arg0;dir1 arg1;");
SrsConfDirective conf;
HELPER_ASSERT_SUCCESS(conf.parse(&buf));
EXPECT_EQ(0, (int)conf.name.length());
EXPECT_EQ(0, (int)conf.args.size());
EXPECT_EQ(2, (int)conf.directives.size());

SrsConfDirective& dir0 = *conf.directives.at(0);
EXPECT_STREQ("dir0", dir0.name.c_str());
EXPECT_EQ(1, (int)dir0.args.size());
EXPECT_STREQ("arg0", dir0.arg0().c_str());
EXPECT_EQ(0, (int)dir0.directives.size());

SrsConfDirective& dir1 = *conf.directives.at(1);
EXPECT_STREQ("dir1", dir1.name.c_str());
EXPECT_EQ(1, (int)dir1.args.size());
EXPECT_STREQ("arg1", dir1.arg0().c_str());
EXPECT_EQ(0, (int)dir1.directives.size());
}
}

VOID TEST(ConfigDirectiveTest, Parse2SingleDirs)
{
srs_error_t err;
Expand Down Expand Up @@ -829,30 +940,7 @@ VOID TEST(ConfigDirectiveTest, ParseLine4)

MockSrsConfigBuffer buf("dir0 {\n\ndir1 \n\narg0;dir2 arg1;}");
SrsConfDirective conf;
HELPER_ASSERT_SUCCESS(conf.parse(&buf));
EXPECT_EQ(0, (int)conf.name.length());
EXPECT_EQ(0, (int)conf.args.size());
EXPECT_EQ(1, (int)conf.directives.size());

SrsConfDirective& dir0 = *conf.directives.at(0);
EXPECT_STREQ("dir0", dir0.name.c_str());
EXPECT_EQ(0, (int)dir0.args.size());
EXPECT_EQ(2, (int)dir0.directives.size());
EXPECT_EQ(1, (int)dir0.conf_line);

SrsConfDirective& dir1 = *dir0.directives.at(0);
EXPECT_STREQ("dir1", dir1.name.c_str());
EXPECT_EQ(1, (int)dir1.args.size());
EXPECT_STREQ("arg0", dir1.arg0().c_str());
EXPECT_EQ(0, (int)dir1.directives.size());
EXPECT_EQ(3, (int)dir1.conf_line);

SrsConfDirective& dir2 = *dir0.directives.at(1);
EXPECT_STREQ("dir2", dir2.name.c_str());
EXPECT_EQ(1, (int)dir2.args.size());
EXPECT_STREQ("arg1", dir2.arg0().c_str());
EXPECT_EQ(0, (int)dir2.directives.size());
EXPECT_EQ(5, (int)dir2.conf_line);
HELPER_ASSERT_FAILED(conf.parse(&buf));
}

VOID TEST(ConfigDirectiveTest, ParseLineNormal)
Expand Down