Skip to content

Commit 515d1d7

Browse files
committed
Accept INI config files that open with a comment
readOrMigrate routed anything not starting with '[' to the legacy "host token" path, so a valid INI file whose first line was a comment failed the host check and was rejected as corrupt, locking the user out of every command. Detect INI by parsing, since parseIni already skips comment and blank lines, and fall back to the legacy format only when no sections are found.
1 parent 583b6ea commit 515d1d7

2 files changed

Lines changed: 40 additions & 22 deletions

File tree

components/configuration/profiles.go

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -283,28 +283,27 @@ func readOrMigrate(data []byte) (*iniFile, bool, error) {
283283
return newIniFile(), false, nil
284284
}
285285

286-
// Old format: no section headers. Check prefix rather than Contains so that
287-
// IPv6 host URLs (e.g. http://[::1]) are not mis-detected as INI files.
288-
if !strings.HasPrefix(content, "[") {
289-
clean := common.SanitizeLineBreaks(content)
290-
parts := strings.SplitN(clean, " ", 2)
291-
// Require the first field to look like a real host URL, so a corrupt
292-
// file is not migrated into bogus credentials just because it happens
293-
// to contain a space.
294-
if len(parts) == 2 && looksLikeHost(parts[0]) && parts[1] != "" {
295-
f := newIniFile()
296-
f.set(DefaultProfile, "host", parts[0])
297-
f.set(DefaultProfile, "token", parts[1])
298-
return f, true, nil
299-
}
300-
return nil, false, invalidConfigError()
301-
}
302-
303-
f := parseIni(data)
304-
if len(f.sections) == 0 {
305-
return nil, false, invalidConfigError()
306-
}
307-
return f, false, nil
286+
// Detect INI by parsing rather than a raw prefix check: parseIni skips
287+
// comment and blank lines, so a valid INI file that opens with a comment
288+
// still yields section headers. Any section present means INI format.
289+
if f := parseIni(data); len(f.sections) > 0 {
290+
return f, false, nil
291+
}
292+
293+
// Old format: no section headers, so treat the content as a single
294+
// "host token" line. Require the first field to look like a real host URL,
295+
// so a corrupt file is not migrated into bogus credentials just because it
296+
// happens to contain a space. looksLikeHost accepts IPv6 host URLs
297+
// (e.g. http://[::1]).
298+
clean := common.SanitizeLineBreaks(content)
299+
parts := strings.SplitN(clean, " ", 2)
300+
if len(parts) == 2 && looksLikeHost(parts[0]) && parts[1] != "" {
301+
f := newIniFile()
302+
f.set(DefaultProfile, "host", parts[0])
303+
f.set(DefaultProfile, "token", parts[1])
304+
return f, true, nil
305+
}
306+
return nil, false, invalidConfigError()
308307
}
309308

310309
func invalidConfigError() error {

components/configuration/profiles_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,3 +382,22 @@ func TestMigration_oldFormatRewrittenAsIni(t *testing.T) {
382382
t.Errorf("after migration: expected [default], got %v", profiles)
383383
}
384384
}
385+
386+
// A valid INI file may open with a comment line. parseIni skips comments, so
387+
// the reader must treat it as INI rather than mistaking it for the legacy
388+
// "host token" format and rejecting it as corrupt.
389+
func TestReadConfig_IniWithLeadingComment(t *testing.T) {
390+
setupTempConfig(t)
391+
writeRaw(t, "# my openproject config\n[default]\nhost = https://commented.example.com\ntoken = commenttoken\n")
392+
393+
host, token, err := configuration.ReadConfig("default")
394+
if err != nil {
395+
t.Fatalf("ReadConfig returned error for commented INI: %v", err)
396+
}
397+
if host != "https://commented.example.com" {
398+
t.Errorf("host = %q, want %q", host, "https://commented.example.com")
399+
}
400+
if token != "commenttoken" {
401+
t.Errorf("token = %q, want %q", token, "commenttoken")
402+
}
403+
}

0 commit comments

Comments
 (0)