Bug in Spring.Utils.TEnvironment.ExpandEnvironmentVariables

[Zuo Baoquan]

The legacy code:

class function TEnvironment.ExpandEnvironmentVariables(
  const variable: string): string;
var
  len: Cardinal;
begin
  len := MAX_PATH;
  SetLength(Result, len);
  len := Windows.ExpandEnvironmentStrings(PChar(variable), PChar(Result), len);
  Win32Check(len > 0);
  SetLength(Result, len - 1);  // BUG
end;

Problems: * ExpandEnvironmentStrings should be invoked again when the buffer is not big enough * The MAX_PATH is confusing * The param name "variable" is confusing * SetLength is always invoked

A possible fix: (Also renamed the param)

class function TEnvironment.ExpandEnvironmentVariables(const s: string): string;
var
  len: Cardinal;
begin
  if s = '' then 
    Exit('');

  SetLength(Result, Length(s));
  len := Windows.ExpandEnvironmentStrings(PChar(s), PChar(Result), Length(Result) + 1);
  Win32Check(len > 0);

  // Environment Variables might be changed during the execution
  while len > Length(Result) + 1 do
  begin
    SetLength(Result, len - 1);
    len := Windows.ExpandEnvironmentStrings(PChar(s), PChar(Result), Length(Result) + 1);
    Win32Check(len > 0);
  end;

  if len < Length(Result) + 1 then
    SetLength(Result, len - 1);
end;

[Stefan Glienke]

fixed issue #98

→ <<cset 54677f09c2f6>>