Skip to content

fix: taskrc parser now correctly preserves values containing '=' characters #612

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

Open
Varadraj75 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: taskrc parser now correctly preserves values containing '=' characters #612

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
23d863a
fix: taskrc parser now correctly preserves values containing '=' char…
Varadraj75 Mar 6, 2026
65a1d70
fix: store sep index in variable, add regression tests for = in values
Varadraj75 Mar 6, 2026
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
13 changes: 8 additions & 5 deletions lib/app/utils/taskserver/parse_taskrc.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
/// Parses a taskrc file into a Dart [Map].
/// Splits each line on the first '=' only, preserving values that
/// contain '=' characters (e.g. base64-encoded certificates, taskd
/// parameters with padding).
Map<String, String> parseTaskrc(String contents) => {
for (var pair in contents
for (var line in contents
.split('\n')
.where((line) => line.contains('=') && line[0] != '#')
.map((line) => line.replaceAll('\\/', '/')) // ignore: use_raw_strings
.map((line) => line.split('=')))
pair[0].trim(): pair[1].trim(),
};
.map((line) => line.replaceAll('\\/', '/'))) // ignore: use_raw_strings
line.substring(0, line.indexOf('=')).trim():
line.substring(line.indexOf('=') + 1).trim(),
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

The core bug fix is correct — using indexOf('=') instead of split('=') properly preserves values containing = characters. However, line.indexOf('=') is called twice on lines 10–11: once to compute the key's end boundary and once to compute the value's start. While the .where filter on line 8 guarantees indexOf will never return -1, calling it twice is a minor inefficiency and a maintainability concern. A local variable such as final sep = line.indexOf('=') should be introduced before the map entry to avoid the repeated call and make the intent clearer.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

@Varadraj75 Varadraj75 Mar 6, 2026

Choose a reason for hiding this comment

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

Fixed — indexOf('=') result is now stored in a local variable `sep` 
to avoid the double call. Regression tests added in the same commit.

Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

The PR checklist explicitly marks "Tests have been added or updated to cover the changes" as unchecked, and the existing test file (test/utils/taskserver/parse_taskrc_test.dart) has no test case for the exact scenario this fix addresses — a value containing one or more = characters (e.g. a base64-encoded certificate like taskd.certificate=abc123==). Without such a test, a regression could silently reintroduce the original bug. A test case like:

key=value==

expecting {'key': 'value=='} should be added.

Copilot uses AI. Check for mistakes.
};
Loading