Skip to content

Commit f5b761a

Browse files
committed
Fix nullptr crash in RecConfigOverrideFromEnvironment with runroot
When runroot is active and the PROXY_CONFIG_* environment variables for path records (bin_path, local_state_dir, logfile_dir, plugin_dir) are not set, RecConfigOverrideFromEnvironment() returned nullptr. This violated its documented contract ("return either the overridden value, or the original value") and caused a std::logic_error crash during records.yaml parsing: basic_string: construction from null is not valid The nullptr was assigned to std::string in SetRecordFromYAMLNode(), which threw the exception. The bug was masked in autests because the framework always sets these env vars. Fix: return the original config value instead of nullptr when RecConfigOverrideFromRunroot() matches. The record value is then resolved by Layout::relative() at runtime, producing the correct path. Add a test that unsets the 4 path env vars to exercise the runroot code path, and validates record values via traffic_ctl config get.
1 parent 00604e1 commit f5b761a

2 files changed

Lines changed: 109 additions & 1 deletion

File tree

src/records/RecConfigParse.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ RecConfigOverrideFromEnvironment(const char *name, const char *value)
123123
if (envval) {
124124
return envval;
125125
} else if (RecConfigOverrideFromRunroot(name)) {
126-
return nullptr;
126+
return value;
127127
}
128128

129129
return value;
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
'''
2+
'''
3+
# Licensed to the Apache Software Foundation (ASF) under one
4+
# or more contributor license agreements. See the NOTICE file
5+
# distributed with this work for additional information
6+
# regarding copyright ownership. The ASF licenses this file
7+
# to you under the Apache License, Version 2.0 (the
8+
# "License"); you may not use this file except in compliance
9+
# with the License. You may obtain a copy of the License at
10+
#
11+
# http://www.apache.org/licenses/LICENSE-2.0
12+
#
13+
# Unless required by applicable law or agreed to in writing, software
14+
# distributed under the License is distributed on an "AS IS" BASIS,
15+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
# See the License for the specific language governing permissions and
17+
# limitations under the License.
18+
19+
Test.Summary = '''
20+
Verify that records.yaml parsing does not crash when runroot is active and
21+
path records are present without corresponding environment variables.
22+
23+
Additionally, validate that:
24+
1. Env var overrides work correctly (record value comes from env).
25+
2. Runroot-managed path records use config file values when their env vars
26+
are unset (the bug scenario).
27+
'''
28+
29+
ts = Test.MakeATSProcess("ts")
30+
31+
# Set an env var override for debug tags. This env var is NOT set by the
32+
# test framework, so it specifically tests the env override path in
33+
# RecConfigOverrideFromEnvironment().
34+
ts.Env['PROXY_CONFIG_DIAGS_DEBUG_TAGS'] = 'from_env_override'
35+
36+
# Set a config value for tags that will be overridden by the env var above.
37+
ts.Disk.records_config.update('''
38+
diags:
39+
debug:
40+
enabled: 0
41+
tags: from_config_value
42+
''')
43+
44+
# Add the runroot-managed path records. Values must match the sandbox layout.
45+
ts.Disk.records_config.append_to_document(
46+
'''
47+
bin_path: bin
48+
local_state_dir: runtime
49+
log:
50+
logfile_dir: log
51+
plugin:
52+
plugin_dir: plugin
53+
''')
54+
55+
# Unset the 4 path env vars to exercise the runroot code path in
56+
# RecConfigOverrideFromEnvironment(). In real deployments (e.g.
57+
# traffic_server --run-root=/path), these env vars are NOT set.
58+
# The test framework always sets them, which masks the bug.
59+
original_cmd = ts.Command
60+
ts.Command = (
61+
"env"
62+
" -u PROXY_CONFIG_BIN_PATH"
63+
" -u PROXY_CONFIG_LOCAL_STATE_DIR"
64+
" -u PROXY_CONFIG_LOG_LOGFILE_DIR"
65+
" -u PROXY_CONFIG_PLUGIN_PLUGIN_DIR"
66+
f" {original_cmd}")
67+
68+
# Test 0: Start ATS
69+
tr = Test.AddTestRun("Start ATS and verify records parsing")
70+
tr.Processes.Default.Command = 'echo 1'
71+
tr.Processes.Default.ReturnCode = 0
72+
tr.Processes.Default.StartBefore(ts)
73+
tr.StillRunningAfter = ts
74+
75+
# Verify no basic_string crash.
76+
ts.Disk.traffic_out.Content = Testers.ExcludesExpression(
77+
"basic_string", "records.yaml parsing must not crash with 'basic_string: construction from null'")
78+
ts.Disk.traffic_out.Content += Testers.ContainsExpression(
79+
"records parsing completed", "ATS should complete records parsing without errors")
80+
81+
# Test 1: Verify env var override works for diags.debug.tags
82+
tr = Test.AddTestRun("Verify env var override for diags.debug.tags")
83+
tr.Processes.Default.Command = 'traffic_ctl config get proxy.config.diags.debug.tags'
84+
tr.Processes.Default.Env = ts.Env
85+
tr.Processes.Default.ReturnCode = 0
86+
tr.StillRunningAfter = ts
87+
tr.Processes.Default.Streams.stdout = Testers.ContainsExpression(
88+
'proxy.config.diags.debug.tags: from_env_override', 'Record should have the env var override value, not the config file value')
89+
90+
# Test 2: Verify runroot-managed path records have their config file values
91+
tr = Test.AddTestRun("Verify path records have config file values")
92+
tr.Processes.Default.Command = (
93+
'traffic_ctl config get'
94+
' proxy.config.bin_path'
95+
' proxy.config.local_state_dir'
96+
' proxy.config.log.logfile_dir'
97+
' proxy.config.plugin.plugin_dir')
98+
tr.Processes.Default.Env = ts.Env
99+
tr.Processes.Default.ReturnCode = 0
100+
tr.StillRunningAfter = ts
101+
tr.Processes.Default.Streams.stdout += Testers.ContainsExpression(
102+
'proxy.config.bin_path: bin', 'bin_path should have the config file value')
103+
tr.Processes.Default.Streams.stdout += Testers.ContainsExpression(
104+
'proxy.config.local_state_dir: runtime', 'local_state_dir should have the config file value')
105+
tr.Processes.Default.Streams.stdout += Testers.ContainsExpression(
106+
'proxy.config.log.logfile_dir: log', 'logfile_dir should have the config file value')
107+
tr.Processes.Default.Streams.stdout += Testers.ContainsExpression(
108+
'proxy.config.plugin.plugin_dir: plugin', 'plugin_dir should have the config file value')

0 commit comments

Comments
 (0)