Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

icmp echo request and response automatically adds 8 bytes to the payload with a tick count #498

Open
SeanSolberg opened this issue Sep 8, 2023 · 3 comments
Labels
Element: ICMP Issues related to TIdICMPClient Status: Deferred Issue to be re-reviewed in a future release Type: Enhancement Issue is proposing a new feature/enhancement

Comments

@SeanSolberg
Copy link

We are unable to use the ICMP client component for our task because we need to be able to do a ping (ICMP Request and Response) with a data payload of 1 bytes However, internal to the the IdICMPClient class within the PrepareEchoRequestIPv4(const ABuffer: String); method, it will include an 8 byte value that comes from the tick counter. This means if we pass in a buffer of 1 character, then
the data payload that is actually sent is 9 bytes.

This internal mechanism should allow us to build the actual payload instead of forcing 8 bytes plus the bytes from the string characters. Further, because nothing internal to this class is virtual, we cannot override this behavior easily with a descendant class. Also, there should be a method to do a ping with a binary array of data payload and not a string that then internally must be converted to the array of bytes payload.

@SeanSolberg SeanSolberg added Status: Reported Issue has been reported for review Type: Bug Issue is a bug in existing code labels Sep 8, 2023
@rlebeau rlebeau added Type: Enhancement Issue is proposing a new feature/enhancement Element: ICMP Issues related to TIdICMPClient and removed Type: Bug Issue is a bug in existing code labels Sep 9, 2023
@rlebeau
Copy link
Member

rlebeau commented Sep 9, 2023

The tick counter is put in the payload to help calculate a value for the ReplyStatus.MsRoundTripTime property when the echo response comes back. I suppose the internal FStartTime member could be used for that purpose instead.

Allowing users to send a binary payload instead of a string payload is a good idea, especially since it doesn't appear the client returns that response data back to the caller anyway.

@SeanSolberg
Copy link
Author

rlebeau, thanks for your comments. We just re-wrote it internally to do what we needed to do.

@rlebeau
Copy link
Member

rlebeau commented Sep 12, 2023

I updated TIdIcmpClient to support TIdBytes for user-defined data. I have not removed the tick counter from the ICMP payload yet, though.

@rlebeau rlebeau added Status: Deferred Issue to be re-reviewed in a future release and removed Status: Reported Issue has been reported for review labels Jul 16, 2024
@rlebeau rlebeau added this to the Indy 11 - Maintenance Release milestone Jul 16, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Element: ICMP Issues related to TIdICMPClient Status: Deferred Issue to be re-reviewed in a future release Type: Enhancement Issue is proposing a new feature/enhancement
Projects
None yet
Development

No branches or pull requests

2 participants