Why doesn't TPageProducer remove quotation marks from strings?

Question

I'm trying to debug behaviour that has only appeared when my large app - working fine in XE3 - is run after compiling with XE4. The issue seems to cause some quoted strings (eg "MyString") to retain their quotes even after having been 'de-quoted' by TPageProducer in Web.HTTPProd. For example, consider the code below which is small extract from this Delphi source unit Web.HTTPApp:

procedure ExtractHeaderFields(Separators, _WhiteSpace: TSysCharSet; Content: PChar;
  Strings: TStrings; Decode: Boolean; StripQuotes: Boolean = False);
{$ENDIF NEXTGEN}
var
  Head, Tail: PChar;
  EOS, InQuote, LeadQuote: Boolean;
  QuoteChar: Char;
  ExtractedField: string;
{$IFNDEF NEXTGEN}
  WhiteSpaceWithCRLF: TSysCharSet;
  SeparatorsWithCRLF: TSysCharSet;
{$ENDIF !NEXTGEN}

  function DoStripQuotes(const S: string): string;
  var
    I: Integer;
    InStripQuote: Boolean;
    StripQuoteChar: Char;
  begin
    Result := S;
    InStripQuote := False;
    StripQuoteChar := #0;
    if StripQuotes then
    begin
      for I := Result.Length - 1 downto 0 do
        if Result.Chars[I].IsInArray(['''', '"']) then
          if InStripQuote and (StripQuoteChar = Result.Chars[I]) then
          begin
            Result.Remove(I, 1);
            InStripQuote := False;
          end
          else if not InStripQuote then
          begin
            StripQuoteChar := Result.Chars[I];
            InStripQuote := True;
            Result.Remove(I, 1);
          end
    end;
  end;

I see this called when I use TPageProducer and I can see my good source string go into the ExtractHeaderFields routine above and then into the 'DoStripQuotes' function. Stepping into DoStripQuotes and watching 'Result' shows that it does not change, even when Result.Remove is called (to strip the quote). When I take this 'DoStripQuotes' routine out to a simple test app, it wont compile, telling me that 'Result.anything' is not allowed. I assume then that Result, although it is defined as 'string' must be another type of string in the context of Web.HTTPProd.

So I get to thinking maybe this is something to do with the 'Immutable strings' that I've heard about. I read this SO question about that and although I get the gist, I could do with more practical advice.

Specifically, I would like answers to the following questions:

1. What type of 'string' is 'Result' if the notation Result.Length is allowed?
2. Is there a way in which I can tell the compiler to use 'XE3' compatibility for a unit? (THis might allow me to see where the problem is originiating). Ive ttried {$ZEROBASEDSTRINGS ON} / OFF but this seems to cause even more chaos and I don't know what I'm doing!

Thanks for any help.

LATER EDIT: As noted in the accepted answer below this is a bug in the VCL unit Web.HTTPApp.pas which should read "Result := Result.Remove(I,1)" in two places around line 2645 and not "Result.Remove(I,1)"

Answer 1

What type of 'string' is 'Result' if the notation Result.Length is allowed?

It's just the same old string, aliased to UnicodeString, that you've been using since Delphi 2009. The difference is that this code uses the new record helper (specifically SysUtils.TStringHelper). That's what lets you use . notation on a string variable.

Is there a way in which I can tell the compiler to use 'XE3' compatibility for a unit?

No. The code in question is a library unit and it is designed to be compiled in a particular mode. What's more, you can't readily re-compile it unless you take on compiling the RTL/VCL yourself. Even if there was such a mode, it would not help since the code is simply wrong (see below). No amount of mode switching can fix this particular piece of code.

I get to thinking maybe this is something to do with the Immutable strings that I've heard about.

It's not. None of the Delphi compilers have immutable strings yet. The concept of immutable strings is just something that has been floated as a future change. And if the change is made, expect it to be made in the mobile compilers first.

---

The problem is in fact just a rather simple bug in the code that you posted which has clearly had no testing at all. The use of Remove is wrong. That method does not modify the string in-place. Instead it returns a new string that has the character removed. The code should read:

Result := Result.Remove(I, 1);

The reason that the developer who coded ExtractHeaderFields has made this mistake is that whoever designed the string helper code named the Remove method incorrectly. Since Remove is a verb you would expect it to operate in-place. A method that does not modify the subject, and returns a new instance, as this method does, should be given a name that is a noun. So this method should be named something like Remnants. It looks to me as though the RTL designers copied the .net naming where the same flaw also exists.

You should submit a QC report, if one does not already exist. I know that XE4 update 1 has just been released. It's plausible that it contains a fix.

Your other options, as I see them, are:

1. Stick with XE3 until XE4 is sufficiently debugged.
2. Include a copy of the Web.HTTPApp unit in your project and fix the bugs yourself.