Rio has a broken REST Library

[Jacek Laskowski 57]

The REST Library in Delphi Rio has a serious error. I was last weekend at the world's largest HackYeah hackathon, where I needed to use the Delphi Rio to connect to the rest service. However, I could not. REST Debbuger did not return any error or response after calling the request (button Send Request). I lost a lot of time on the verification of network connections and the operation of the server itself (I thought that a heavy load on the hackatone participants "killed" the server) but it turned out to be a mistake in the REST Library. When I tried to call the request directly from Delphi (request.Execute ()) I got an error in KERNELBASE. There is no error in Delphi Tokyo, the same code worked there, but on my laptop I only had Rio. So I was looking for a solution. Eventually, I found the place and the reason, corrected RTL file (after copy to project directory) and the code runed, but the bad taste remained.

Later I will publish where the error lies.

[Kryvich 165]

Weird. Waiting for your findings. The good thing is that CE comes with REST sources, so a competent enthusiast can help find a bug in the library.

[Jacek Laskowski 57]

Bug exist in System.Net.HttpClient.Win.pas file. This is not exactly REST Library like I wrote previous, but REST uses this subsystem too.

 

Bad function with my changes:

 

function ReadHeader(ARequest: HINTERNET; AHeaderFlag: DWORD; const AHeaderName: string = ''): string;
var
  LSize: Cardinal;
  LFlags: DWORD;
  LHeaderName: PWideChar;
begin
  LFLags := AHeaderFlag;
  if AHeaderName <> '' then
  begin
    LFLags := LFLags or WINHTTP_QUERY_CUSTOM;
    LHeaderName := PWideChar(AHeaderName);
  end
  else
    LHeaderName := WINHTTP_HEADER_NAME_BY_INDEX;

  LSize := 0;
  WinHttpQueryHeaders(ARequest, LFLags, LHeaderName, nil, LSize, WINHTTP_NO_HEADER_INDEX);

  if GetLastError = ERROR_WINHTTP_HEADER_NOT_FOUND then
    Result := ''
  else
  begin
    if GetLastError <> ERROR_INSUFFICIENT_BUFFER then
      raise ENetHTTPException.CreateResFmt(@SNetHttpHeadersError, [GetLastError, SysErrorMessage(GetLastError, TWinHttpLib.Handle)]);

    SetLength(Result, LSize div SizeOf(Char) - 1);
    
    // ----------------------  my changes begin
    if Length(Result) = 0 then
    begin
      SetLength(Result, 1);
    end;
    // ---------------------- my changes end
    
    if WinHttpQueryHeaders(ARequest, LFLags, LHeaderName, PChar(Result), LSize, WINHTTP_NO_HEADER_INDEX) = False then
      raise ENetHTTPException.CreateResFmt(@SNetHttpHeadersError, [GetLastError, SysErrorMessage(GetLastError, TWinHttpLib.Handle)]);
  end;
end;

The problem occurs when a zero-length string (SetLength) is created and passed by the PChar to the WinHttpQueryHeaders() method as Nil.

 

SetLength(Result, LSize div SizeOf(Char) - 1);  //when LSize = 2 then Result is empty string

 

 

I can't reproduce this case now, because I don't have access to that site that caused the error on the hackathon, and on a few other ones that I checked right now, this situation doesn't occur (the header row with 2 characters).

 

Edited November 26, 2018 by Jacek Laskowski

[Attila Kovacs 629]

https://docs.microsoft.com/en-us/windows/desktop/api/winhttp/nf-winhttp-winhttpqueryheaders

 

• If the function fails and ERROR_INSUFFICIENT_BUFFER is returned, lpdwBufferLengthspecifies the number of bytes that the application must allocate to receive the string.

 

Looks like the -1 is the problem.

 

 

Edited November 26, 2018 by Attila Kovacs

[Kryvich 165]

Before the second call of WinHttpQueryHeaders there is a check:

if GetLastError <> ERROR_INSUFFICIENT_BUFFER then
      raise ENetHTTPException.CreateResFmt(...);

It means that the call of function WinHttpQueryHeaders occurs only if GetLastError = ERROR_INSUFFICIENT_BUFFER. According to the API:

• If the function fails and ERROR_INSUFFICIENT_BUFFER is returned, lpdwBufferLengthspecifies the number of bytes that the application must allocate to receive the string.

[Stefan Glienke 2002]

@Attila Kovacs Your quote from the help is kinda misleading because the code path there is the one handling the function not succeeding but returning ERROR_INSUFFICIENT_BUFFER.

 

@Kryvich The help combined with the code below is confusing me. It says number of bytes but then it also does a dwSize/sizeof(WCHAR) to allocate a wchar array 😕

Edited November 26, 2018 by Stefan Glienke

[Attila Kovacs 629]

@Stefan Glienke I've seen too late, already updated, also not sure anymore, but what else could be

Edited November 26, 2018 by Attila Kovacs

[Kryvich 165]

As I understand WinHttpQueryHeaders should return an Unicode string, so LSize should be even. But we can foresee any variant, allocating enough space for the result string:

SetLength(Result, (LSize+1) div SizeOf(Char));

 

Edited November 26, 2018 by Kryvich

[Attila Kovacs 629]

I like winapi. There you can get real knowhow by bruteforcing it.

[Attila Kovacs 629]

Ok, looks like the coder used this trick to trim the trailing zeroes but didn't count with empty/short headers.

[David Schwartz 426]

That'll teach you to avoid using a fresh-out-of-the-oven compiler release on a time-sensitive project!

 

In this case, I'd have stuck with what I had the most confidence in. So your hackathon experience turned into a bug-a-thon for EMBT. 🙂

[Dmitry Arefiev 101]

Do we have a report for this ?

Edited November 27, 2018 by Dmitry Arefiev

[Sherlock 663]

Yes, please!

[Jacek Laskowski 57]

1 hour ago, Dmitry Arefiev said:

Do we have a report for this ? 

https://quality.embarcadero.com/browse/RSP-21770