Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix for GitHub issue #377 #378

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Conversation

auerswal
Copy link
Collaborator

@auerswal auerswal commented Feb 1, 2025

This pull request has two commits:

  1. Additional tests for useful features of the current code that reads target names from file (or standard input).
  2. A fix for the reported issue, i.e., splitting long lines, and not supporting long, but possibly valid, DNS names.

* whitespace in sufficiently short lines is skipped
* additional words after the target are ignored in sufficiently
  short lines
* whitespace-only (i.e., blank, but not empty) lines are skipped
* DOS-format text files, i.e., lines terminated with CRLF, work
* input need not be a correct POSIX text file, i.e., non-empty
  input does not need to end with LF
@coveralls
Copy link

coveralls commented Feb 1, 2025

Coverage Status

coverage: 87.692% (+0.08%) from 87.611%
when pulling ff1aa04 on auerswal:issue377
into 511aa37 on schweikert:develop.

@auerswal auerswal linked an issue Feb 1, 2025 that may be closed by this pull request
@auerswal auerswal marked this pull request as draft February 6, 2025 09:18
@auerswal
Copy link
Collaborator Author

auerswal commented Feb 6, 2025

There are at least two bugs still lurking in the new code. I plan to add the test cases below, and fix the behavior. For now, I have converted the pull request to a "draft".

When the first part of the line read into the buffer ends with the target name, and the first character of the second part of the line is a '#', then the data from the second line part is ignored instead of added to the target name.

Observed behavior:

$ perl -e 'for (my $i = 1; $i < 4; $i++) { print " "x(255-11+$i)."127.0.0.$i#host.name.invalid\n" }' | ./src/fping 
127.0.0.1#host.name.invalid: Name or service not known
127.0.0.3#host.name.invalid: Name or service not known
127.0.0.2 is alive

Expected behavior:

$ perl -e 'for (my $i = 1; $i < 4; $i++) { print " "x(255-11+$i)."127.0.0.$i#host.name.invalid\n" }' | ./src/fping 
127.0.0.1#host.name.invalid: Name or service not known
127.0.0.2#host.name.invalid: Name or service not known
127.0.0.3#host.name.invalid: Name or service not known

Then there is the problem that a comment in the first part of the line still results in splitting long lines.

Observed behavior:

$ perl -e 'print "#"." "x300 ."localhost\n"' | ./src/fping 
localhost is alive

Expected behavior:

$ perl -e 'print "#"." "x300 ."localhost\n"' | ./src/fping 

This addresses GitHub isse schweikert#377.

Before, input lines longer than the static buffer used to read
data would be handled as if each buffer part were one line.
This could result in splitting input target names into multiple
different target names.

Since the static buffer was significantly smaller then the maximum
length of DNS names (just short of 255 one octet characters),
valid long DNS names could not be read as targets via file.

This commit fixes the splitting of long lines, and also increases
the maximum length of target names to 255 one octet characters.

The additional line parsing code is kept similar to the existing
code and attempts to keep all intended and/or useful features to
minimize the risk of regressions.
@auerswal auerswal marked this pull request as ready for review February 8, 2025 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Splitting with a long URL and a pipe
2 participants