Überlauf bei int64

[Hobbycoder]

Hi,

in folgernder function möchte ich aus einer IP-Adresse einen Int64 machen.
Doch beim berechnen des Results kommt es zu einem EIntOverflow / Integerüberlauf. Wie kann das sein? Int64 deckt das doch locker ab.

Delphi-Quellcode:

function IP2Int(value: string): Int64;
var
  s, v1, v2, v3, v4: string;
  i1, i2, i3, i4: Integer;
begin
  Result:=0;
  s:=value;
  if Pos('.', s)>0 then
  begin
    v1:=Copy(s, 1, pos('.', s)-1);
    s:=Copy(s, Pos('.', s)+1, Length(s));
    if Pos('.', s)>0 then
    begin
      v2:=Copy(s, 1, pos('.', s)-1);
      s:=Copy(s, Pos('.', s)+1, Length(s));
      if Pos('.', s)>0 then
      begin
        v3:=Copy(s, 1, pos('.', s)-1);
        V4:=Copy(s, Pos('.', s)+1, Length(s));
      end;
    end;
  end;
  if TryStrToInt(v1, i1) then
    if TryStrToInt(v2, i2) then
      if TryStrToInt(v3, i3) then
        if TryStrToInt(v4, i4) then
        begin
          Result:=i1 * 16777246 +    //<----Hier tritt der Überlauf auf.
                  i2 * 65536    +
                  i3 * 256      +
                  i4;
        end;
end;

[dummzeuch]

Der Ausdruck wird zuerst als Integer ausgewertet und erst dann zugewiesen. Du musst mindestens einen der Summanden als Int64 typecasten (oder gleich also solchen deklarieren), dann wird es funktionieren:

Delphi-Quellcode:

  Result:=Int64(i1) * 16777246 +
          i2 * 65536 +
          i3 * 256 +
          i4;

[dummzeuch]

Ich habe das mal Claude vorgelegt und der hat noch einen Fehler gefunden:

Zitat:

The most significant error is in the first multiplier: 16777246. This number appears to be incorrect. For converting IP addresses to integers, the correct multiplier should be 16777216 (which is 256³). The current value of 16777246 is off by 30, which can lead to incorrect results.

Und er gibt noch folgende Ratschläge:

Zitat:

To make the function more robust, you might also want to:

• Add range checking to ensure each octet is between 0 and 255
• Add error handling for malformed IP addresses
• Consider using Cardinal or UInt32 instead of Int64 since IPv4 addresses fit in 32 bits

Alles durchaus sinnvoll.

Das eigentliche Problem hat er allerdings in seinem "korrigierten" Code nicht gelöst:

Delphi-Quellcode:

function IP2Int(value: string): Int64;
var
  s, v1, v2, v3, v4: string;
  i1, i2, i3, i4: Integer;
begin
  Result := 0;
  s := value;
  
  if Pos('.', s) > 0 then
  begin
    v1 := Copy(s, 1, pos('.', s)-1);
    s := Copy(s, Pos('.', s)+1, Length(s));
    if Pos('.', s) > 0 then
    begin
      v2 := Copy(s, 1, pos('.', s)-1);
      s := Copy(s, Pos('.', s)+1, Length(s));
      if Pos('.', s) > 0 then
      begin
        v3 := Copy(s, 1, pos('.', s)-1);
        v4 := Copy(s, Pos('.', s)+1, Length(s));
      end;
    end;
  end;
  
  if TryStrToInt(v1, i1) then
    if TryStrToInt(v2, i2) then
      if TryStrToInt(v3, i3) then
        if TryStrToInt(v4, i4) then
        begin
          Result := i1 * 16777216 +  // Corrected from 16777246
                   i2 * 65536 +
                   i3 * 256 +
                   i4;
        end;
end;

Der Integer Overflow dürfte immer noch auftreten.

[DeddyH]

Mein Vorschlag wäre

Delphi-Quellcode:

function IPv4StrToInt(const s: string): Cardinal;

  procedure RaiseInvalidIP;
  begin
    raise Exception.Create('Invalid IP');
  end;

var
  lStrOctets: TArray<string>;
  lOctets: array[0..3] of Cardinal;
  i: integer;
begin
  lStrOctets := s.Split(['.']);
  if Length(lStrOctets) <> 4 then
    RaiseInvalidIP;
  for i := Low(lStrOctets) to High(lStrOctets) do
    begin
      if not TryStrToUInt(lStrOctets[i], lOctets[i]) then
        RaiseInvalidIP
      else if not (lOctets[i] in [0..255]) then
        RaiseInvalidIP;
    end;
  Result := lOctets[0] shl 24 or lOctets[1] shl 16 or lOctets[2] shl 8 or lOctets[3];
end;

[jaenicke]

Oder schlicht so:

Delphi-Quellcode:

uses
  Winapi.Winsock;

function IPToCardinal(const AIP: string): Cardinal;
begin
  Result := ntohl(inet_addr(PAnsiChar(AnsiString(AIP))));
end;

Alternativ, wenn es manuell passieren soll (ohne Fehlerbehandlung):

Delphi-Quellcode:

function IPToCardinal2(const AIP: string): Cardinal;
var
  Value, Octet: Cardinal;
  i, ShlValue: Integer;
begin
  Result := 0;
  Value := 0;
  ShlValue := 24;
  for i := 1 to Length(AIP) do
  begin
    if AIP[i] = '.' then
    begin
      Result := Result or (Value shl ShlValue);
      Value := 0;
      ShlValue := ShlValue - 8;
    end
    else
      Value := Value * 10 + (Ord(AIP[i]) - Ord('0'));
  end;
  Result := Result or Value;
end;

[Kas Ob.]

Hi,

Away from the answers above, your code doesn't validate the input for valid IP address and will always result in something, so i suggest to replace the converting part and building with something like this:

Delphi-Quellcode:

    {if TryStrToInt(v1, i1) then
    if TryStrToInt(v2, i2) then
      if TryStrToInt(v3, i3) then
        if TryStrToInt(v4, i4) then
        begin
          Result := i1 * 16777216 + // Corrected from 16777246
                   i2 * 65536 +
                   i3 * 256 +
                   i4;
        end ;  }
  i1 := StrToIntDef(v1, -1);
  i2 := StrToIntDef(v2, -1);
  i3 := StrToIntDef(v3, -1);
  i4 := StrToIntDef(v4, -1);

  if Cardinal(i1 or i2 or i3 or i4) > 255 then      //  bit manipulation if any has higher bit than 8 then invalid (this includes negatives)
    begin
      raise Exception.Create('Invalid IP address');
    end;

  Result := i1 shl 24 or i2 shl 16 or i3 shl 8 or i4;
  // or
  //Result := i1 shl 24 + i2 shl 16 + i3 shl 8 + i4;

[Rollo62]

TLDR;
Es sollten doch die Octets auch auf Überlauf überprüft werden, also so ungefähr:

Edit: Ok, DeddyH hatte die Prüfung auch schon vorgeschlagen ...

Code:

function IP2Int(const value: string): Int64;
var
  parts: TArray<string>;
  i, octet: Integer;
begin
  Result := 0;
  parts := value.Split(['.']);
  if Length(parts) <> 4 then
    Exit;
  for i := 0 to 3 do
  begin
    if not TryStrToInt(parts[i], octet) then
      Exit;
    if (octet < 0) or (octet > 255) then   //<== Check this
      Exit;
    Result := (Result shl 8) + octet;
  end;
end;

[Hobbycoder]

Zitat:

Zitat von dummzeuch (Beitrag 1546453)

Der Ausdruck wird zuerst als Integer ausgewertet und erst dann zugewiesen. Du musst mindestens einen der Summanden als Int64 typecasten (oder gleich also solchen deklarieren), dann wird es funktionieren:

Delphi-Quellcode:

  Result:=Int64(i1) * 16777246 +
          i2 * 65536 +
          i3 * 256 +
          i4;

Dank dir. So geht's.

16777216 ist natürlich korrekt. :-)