function SHA1ofStr or Base64Encode fault output

[xauxag 0]

Hi, Here may be a bug,would you  please have a look?, 

 

unit:  OverbyteIcsWebSocketCli.pas

position: line 1494   ServerKey := Base64Encode(String(SHA1ofStr(s)));

 

------------

test:   x3JJHMbDL1EzLkh9GBhXDw==258EAFA5-E914-47DA-95CA-C5AB0DC85B11

expect: HSmrc0sMlYUkAGmm5OPpG2HaGWk=

output:HSmrc0sMlYUkAGmm5OPpG2E/aQ==

 

------------
test  code:

procedure TForm1.Button3Click(Sender: TObject);
var
   s : AnsiString;
   ServerKey:string;
begin
        s := AnsiString(edit1.Text);
            ServerKey := Base64Encode(String(SHA1ofStr(s)));
            memolog.Lines.Add(ServerKey);
end;

node.js - Base64 encoding for Sec-WebSocket-Accept value - Stack Overflow

Edited January 5 by xauxag

[Angus Robertson 574]

Are you suggesting there is a problem with the Base64Encode function creating incorrect output? 

 

Have you attempted to debug and fix the problem yourself, or are you expecting someone else to do it?

 

I've been testing the OverbyteIcsWebSocketCli unit this week and it works fine. 

 

Angus

[Virgo 18]

Really strange... That output value decodes to hexencoded value

1d29ab734b0c9585240069a6e4e3e91b613f69

Correct value would be

1d29ab734b0c9585240069a6e4e3e91b61da1969

So da19 to 3f or lead surrogate-da19 to question mark? Some sort weird unicode issue?

[xauxag 0]

3 hours ago, Angus Robertson said:

Are you suggesting there is a problem with the Base64Encode function creating incorrect output? 

 

Have you attempted to debug and fix the problem yourself, or are you expecting someone else to do it?

 

I've been testing the OverbyteIcsWebSocketCli unit this week and it works fine. 

 

Angus

Yes I am pretty sure it's a bug, I have debug into  ICS  and found the two function that caused failure of websocket connection ,  and am expecting fixing from more official way .  I just a little time ago found it and  am looking into those code , and not sure exact the correcting  till now  , so any suggestions are  welcome  

Edited January 5 by xauxag

[Angus Robertson 574]

I will need to reproduce connection failures before I look into fixing them.  So I need a Websocket server to test against. 

 

My only suggestion is to try the client against other servers and see if any fail, because no-one else has reported any problems. 

 

Angus

 

[Kas Ob. 121]

5 hours ago, xauxag said:



node.js - Base64 encoding for Sec-WebSocket-Accept value - Stack Overflow

 

I don't see any bug or wrong here, used the same 3 lines like yours and the same value.

 

On other hand if ICS Base64 encoding is broken then it is amazing to perform anything at all !

[Kas Ob. 121]

5 hours ago, xauxag said:

test  code:

procedure TForm1.Button3Click(Sender: TObject);
var
   s : AnsiString;
   ServerKey:string;
begin
        s := AnsiString(edit1.Text);
            ServerKey := Base64Encode(String(SHA1ofStr(s)));
            memolog.Lines.Add(ServerKey);
end;

Please try this instead

procedure TForm1.Button3Click(Sender: TObject);
var
  s: AnsiString;
  ServerKey: string;
begin
  s := AnsiString(edit1.Text);
  ServerKey := Base64Encode(SHA1ofStr(s));
  Memo1.Lines.Add(ServerKey);
end;

 

[David Heffernan 2345]

I'm just curious but why are you using AnsiString casts which produce behaviour dependent on the executing process locale? Did you mean to encode as UTF8? 

[Angus Robertson 574]

Quote

why are you using AnsiString casts

The Base64Encode and SHA1ofStr functions were written 20 to 25 years ago before Unicode, we convert to UTF-8 before calling them if needed.

 

In this unit, the input to create the server key is a Base64 encoded random number we created and sent to the server earlier and a GUID so all simple ASCII.

 

Angus

 

[Brian Evans 105]

The bug is in your code - not all binary data can cleanly be put in a string directly. 

 

The output of the hash function is in TBytes and the encode function can take TBytes as input. That string() cast you inserted between them is causing issues depending on the binary hash value.

[Angus Robertson 574]

Quote

the output of the hash function is in TBytes and the encode function can take TBytes as input.

The output of the hash function is AnsiString and the input to encode is AnsiString, so the String cast is unnecessary.  I'll remove it for the new version going to SVN today.

 

Angus

 

[Angus Robertson 574]

This Websocket client problem should be fixed in SVN and the overnight zip, assuming it was an ANSI/Unicode casting problem with non-English character set conversions, which I'm unable to reproduce.  It did not affect Websocket server which had no casting, and yet always worked against our client. 

 

The real problem is ICS has overloaded versions of many functions for ANSI and Unicode, and the compiler does not always choose the correct version if input and output parameters don't match, fixed by using a specific ANSI function.  I'm slowly adding TBytes versions for binary data to avoid such problems.  but it's a long job. 

 

Angus

 

[Kas Ob. 121]

If @xauxag can provide some more input about the locale and settings then it is possible to test it.

I had being using https://github.com/xupefei/Locale-Emulator for years for the purpose of testing when such problem appear or for testing a specific non internationalized (English) application.

 

[David Heffernan 2345]

On 1/8/2024 at 8:54 AM, Angus Robertson said:

the input to encode is AnsiString

It makes no sense to me that a base64 function could ever accept string input. Input base to be bytes. 

 

On 1/8/2024 at 8:54 AM, Angus Robertson said:

The output of the hash function is AnsiString

This is also kinda weird. Shouldn't it be the native type for text, string. 

[Virgo 18]

Since hash function results binary hash and not hexencoded one and the function predates Unicodestring Ansistring result makes sense. UnicodeString for binary hash would be the worst type.

[David Heffernan 2345]

22 minutes ago, Virgo said:

UnicodeString for binary hash would be the worst type.

I was referring to base64. That takes binary input (so bytes) and outputs text (so string). 

 

For sha1 the yes the input and output should be binary, that is bytes. The you'd have helper functions to take hash output and write in friendly hex format. 

 

At no point should there be AnsiString. I mean I hope you aren't proposing AnsiString as a place to hold bytes. 

Edited January 10 by David Heffernan

[Virgo 18]

2 minutes ago, David Heffernan said:

I mean I hope you aren't proposing AnsiString as a place to hold bytes. 

What I ment was, that I got an impression, that SHA1ofStr and Base64Encode functions were originally written for pre-Unicode Delphi. And to make them work on Unicode Delphi version String was just replaced with AnsiString.  And on old Delphi versions it was usual to use string as byte buffer. Event Delphi itself did it.

[David Heffernan 2345]

29 minutes ago, Virgo said:

And on old Delphi versions it was usual to use string as byte buffer. Event Delphi itself did it.

On modern delphi you can't get away with this as has been discussed many times. 

 

Anyway just because people got things wrong 20 years ago is no reason to continue getting things wrong. 

[Angus Robertson 574]

ICS still supports Delphi 7 and later, so needs AnsiStrings.  I am trying to modernise some code, but can not break old functions that people have used in applications for 20 years.  

 

I'm expecting some bad feedback as people upgrade to ICS V9.1 which has some non-backward compatible SSL/TLS changes, new units and conceptual changes.  SVN notes have all the details. 

 

Angus

 

[David Heffernan 2345]

2 hours ago, Angus Robertson said:

ICS still supports Delphi 7 and later, so needs AnsiStrings. 

I mean, byte arrays exist in Delphi 7

[Angus Robertson 574]

Quote

I mean, byte arrays exist in Delphi 7

Correct, and ICS has always used byte arrays in low level functions, the MD5 digest is a byte array, although the Sha1 digest is an AnsiChar array since it was contributed. 

 

But there were no TBytes library functions in Delphi 20 years old, so AnsiString was commonly used.  I've added a lot of TBytes  functions to ICS in the last few years so that support for old compilers has continued, and a TBytes version of Base64 conversion recently.  But old code only gets modernised when updated for some reason. 

 

Angus

 

[David Heffernan 2345]

1 hour ago, Angus Robertson said:

But old code only gets modernised when updated for some reason. 

There's plenty of reasons not to use AnsiString, unless the library is not developed any more.

[xauxag 0]

On 1/6/2024 at 1:02 AM, Kas Ob. said:

Please try this instead

procedure TForm1.Button3Click(Sender: TObject);
var
  s: AnsiString;
  ServerKey: string;
begin
  s := AnsiString(edit1.Text);
  ServerKey := Base64Encode(SHA1ofStr(s));
  Memo1.Lines.Add(ServerKey);
end;

 

Thanks， It works

19 hours ago, Kas Ob. said:

If @xauxag can provide some more input about the locale and settings then it is possible to test it.

I had being using https://github.com/xupefei/Locale-Emulator for years for the purpose of testing when such problem appear or for testing a specific non internationalized (English) application.

 

It is Chinese simplify