Ü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;