Age | Commit message (Collapse) | Author | Files | Lines |
|
Fix the macOS Intel build (but we should find out why zstd
isn't installed right now.)
|
|
If a null argument is given to a macro, print an error saying that
is disallowed instead of substituting the null argument (i.e., an
unquoted empty string) into the macro.
The latter almost certainly still produces a grammatical error, but it
will be something mysterious that depends on the macro definition like
"==" was unexpected in this context
instead of a useful error string.
For macros that take strings as argument, substituting a null has
never worked either, "" has always needed to be used.
As a special case, accept a single empty argument as meaning
"a macro with 0 arguments" instead of how it is currently treated,
a "macro with 1 null argument", i.e. $mymacro() for the new
function-like syntax and ${mymacro:} for the original syntax.
See 7d87367e22119be4b67822ebe14671a3f56a7f61
|
|
Instead of requiring ${macro:arg1;...;argN}, allow the format
${macro;arg1;...;argN}.
The semicolon isn't used anywhere else, it's simple to support,
and already used in the macro syntax. It's easier to remember
if all the separators in a macro are the same.
The colon is allowed in literals, which is why it's not used
between the arguments in the macro argument list, and allowing
it after the name makes the grammar more complicated, including
tokenizing when having pop-ups of potential field matches in
the display filter line edit (#19499.)
Update the documentation for this. Also edit the documentation
for macro syntax in a few places where it implies that whitespace
in macro arguments would be ignored; in fact, it's significant.
|
|
This is handy when you want to specify a mask in hex format
|
|
|
|
|
|
Return the number of bytes decoded and placed in the tree and
set pinfo->desegment_offset and desegment_len so that the QUIC
disssector can desegment the HTTP3 Encoder stream.
Pass that number of bytes to the nghttp3 decoder so that we don't
end up passing the same bytes twice with reassembly.
Make it so the QUIC data stream desegmenting code puts a link
to the frame data was reassembled in for segments that begin
an MSP as well in more cases, as the TCP dissector does.
(There are a few more cases TODO to produce results similar to
TCP.)
Fix #19475
|
|
|
|
|
|
Disable TestCategory034 since it's failing with the latest automatic
updates.
|
|
Make sure sampleif.py uses the same Python executable as our test
environment. Remove a related workaround from our GitLab CI config.
|
|
Change "[TCP segment of a reassembled PDU]" to
"[TCP PDU reassembled in <frame #>]" in the Packet List.
This fixes a bug where the former message was displayed in cases
where the PDU was not in fact reassembled such as when a frame is
missing from the capture.
TCP: Show the frame in which a PDU is reassembled
Added six checks for "'[TCP PDU reassembled in'" which are necessary
to prevent the display of that info from breaking the testing
commits of others.
These commits are squashed.
|
|
Manual revert of commit 0e82c6b4b8ed18ef1878446dd26d6345be2d2c2b.
Fixes #19493.
|
|
Make sure NN-MM is parsed as subtraction.
Avoid the use of hyphen with bytes unless preceded with a colon
(the prefix for literals).
Before:
Filter:
tcp.port == 64-63
Error: "64-63" cannot be converted to Unsigned integer (16 bits).
tcp.port == 64-63
^~~~~
After:
Filter:
tcp.port == 64-63
Instructions:
0000 READ_TREE tcp.port -> R0
0001 IF_FALSE_GOTO 3
0002 ANY_EQ R0 == 1
0003 RETURN
|
|
Linux and Windows, at least, when performing Local Checksum Offload
during Generic Segmentation Offload and at other times, place the one's
complement sum of the pseudo header in the checksum field, which
provides the necessary correction when a device (or its driver,
if not supported in hardware) computes the one's complement checksum
of each inner layer buffer in the skbuff. (This is why GSO requires
equal length buffers - so that the pseudo header contribution to
the checksum is the same.)
When performing our Internet checksum calculation, we can output
the partial sum of all but the last vector, which is an intermediate
result we calculate in the process anyway. The last vector is
generally the payload, and the previous vectors are for the pseudo
header. We can then compare this partial sum to the value in the
UDP or TCP header if the overall computed checksum isn't 0.
If it matches appropriately, we can have a more informative and
less scary message.
Update the tests a bit because this causes checksums to no longer
fail and be reported malformed in the http2 reassembly example.
Fix #18864. Related to #19109
|
|
Unparsed tokens are tokens that can either be fields
or literal values like byte arrays or something weirder.
Some cases are troublesome, like two letter tokens
being a protocol name or a byte (fc is Fiber Channel or 0xFC),
or hypothetically aa.bb.cc being a byte array
{ 0xaa, 0xbb, 0xcc} or the "bb.cc" field of the "aa"
protocol. Etc.
This semantic difference obviously matters when parsing
an expression and providing helpful error messages to users.
I have now made several attempts at resolving unparsed tokens
into field/not field at the lexical level and still provide
good error messages and there are always limitations
and weird corner cases. Assigning a semantic type to
such ambiguous tokens requires more context. Originally this
was implemented by checking for registered field values
in the scanner but that is one of the possible solutions that
does not produce good results in practice IMO.
Accept that we will never fully fix this without backward
incompatible grammar changes and commit to resolving unparsed
types during the semantic check phase and maybe having a convoluted
lemon grammar with lots of ugly UNPARSED special cases.
|
|
If the quote character appears in a field value, then escape
it by printing the character twice. When escaping whitespace
with the backslash character, also escape the backslash
character itself.
Add a ws_escape_csv function to wsutil and use it for tshark.
Adopt the existing static escape_string_len function so that
ws_escape_csv can use it while maintaining the same output
for the other ws_escape_ functions.
Fix #10284
|
|
Add an alternative macro notation as $mymacro(a,b,c,d). For me
this notation is more natural, I have difficulty remembering how
to use macros with ${mymacro:a;b;c} and it makes the filter
expression harder to understand.
For convenience and to simplify the code we also allow
curly braces to open/close macro argument lists and the semicolon
as an argument separator for the new syntax.
This added flexibility may be reevaluated and dropped later if it
turns out to be undesirable for some reason.
|
|
|
|
As noted in #17923, commit c4731738 broke timereferences and displayed
delta time (`frame.time_delta_displayed`) in the `frames` method of
`sharkd`. This commit adds back to `sharkd_session_process_frames`
the local variables `prev_dis_num`, `current_ref_frame` and
`ref_frame` which are used to determine the `frame_ref_num` and
`prev_dis_num` arguments to each call to `sharkd_dissect_request` in
the main loop of `sharkd_session_process_frames`.
Below is an example on `master` (`b7cc44eb34`, specifically) of a
`frames` request for packets 1 and 800 of the capture
`./test/captures/logistics_multicast.pcapng` where we ask for columns
`frame.time_relative`, `frame.time_delta` and
`frame.time_delta_displayed`:
# ./cmake-build-debug/run/sharkd -
Running as user "root" and group "root". This could be dangerous.
Hello in child.
{"jsonrpc":"2.0","id":1,"method":"load","params":{"file":"./test/captures/logistics_multicast.pcapng"}}
load: filename=./test/captures/logistics_multicast.pcapng
{"jsonrpc":"2.0","id":1,"result":{"status":"OK"}}
{"jsonrpc":"2.0", "id":2, "method":"frames","params":{"filter":"frame.number==1||frame.number==800","column0":"frame.time_relative:1","column1":"frame.time_delta:1","column2":"frame.time_delta_displayed:1"}}
{"jsonrpc":"2.0","id":2,"result":[{"c":["0.000000000","0.000000000","0.000000000"],"num":1},{"c":["191.872111000","0.193716000","0.193716000"],"num":800}]}
Note that the `frame.time_delta_displayed` column value for packet 800
is `0.193716000`, which is time difference between it and packet 799,
not packet 1.
Compare this to the same `frames` request using the changes from this
commit:
# ./cmake-build-debug/run/sharkd -
Running as user "root" and group "root". This could be dangerous.
Hello in child.
{"jsonrpc":"2.0","id":1,"method":"load","params":{"file":"./test/captures/logistics_multicast.pcapng"}}
load: filename=./test/captures/logistics_multicast.pcapng
{"jsonrpc":"2.0","id":1,"result":{"status":"OK"}}
{"jsonrpc":"2.0", "id":2, "method":"frames","params":{"filter":"frame.number==1||frame.number==800","column0":"frame.time_relative:1","column1":"frame.time_delta:1","column2":"frame.time_delta_displayed:1"}}
{"jsonrpc":"2.0","id":2,"result":[{"c":["0.000000000","0.000000000","0.000000000"],"num":1},{"c":["191.872111000","0.193716000","191.872111000"],"num":800}]}
Note that the `frame.time_delta_displayed` column value for packet 800
is now `191.872111000`, the time difference between it and packet 1.
This is the expected value since only packets 1 and 800 are visible
due to the request's `filter` parameter.
A new `test_sharkd_req_frames_delta_times` unit test has been added to
verify this fix and prevent it the bug from being accidentally
sneaking in again. If this fix is accepted, this change should
probably be cherry-picked to the `release-4.2` branch.
Fixes #17923.
|
|
Don't blitz the temporary directory to make test results more
traceable and conform to pytest best practice.
Prefer the pytest method of creating a temporary directory in
the system tmpdir with roll-over, that won't be immediately destroyed.
|
|
Fixes #19466.
|
|
When the jumps_ptr is NULL a nested function call
results in a NULL pointer dereference. We could add
a NULL check but removing the jump in commit e85f8d4cf19
was a mitake, because the jump is not always a no-op,
so add it back.
Fixes e85f8d4cf193721cd95daa15363054cd4d12a0f3.
|
|
Once upon a time, Wireshark could use GNU ADNS instead of c-ares
for asynchronous DNS lookups. GNU ADNS didn't check the system
hosts file (see 51984de040b804ca6614830acc62c641cd6d8959), so
we added the system hosts file using the same mechanism as profile
paths when using ADNS.
This was then confusing, because "use external DNS resolver / use
system DNS resolving" could be off but /etc/hosts was still used,
so the "only use profile hosts" option was created to avoid using
external system DNS hostsnames at all.
c-ares (and, for that matter, libunbound) does read /etc/hosts, so
this option doesn't do its primary purpose anymore. All it usually
does now is keep any hosts file in the global profile from being used,
but we don't have any other name resolution options where there's a
pref not to use global profile data.
Even more confusingly, if the option is true, then the -H option
to tshark to give a hosts file on the command line doesn't work.
add_hosts_file checks the preference and then doesn't actually
read the file from the command line, which is surely never wanted.
Most people don't understand what the option means, despite the
tooltip, and assume that it means "only use the hosts file as a
source of name resolution data", not "when using hosts files as
a source of name resolution data, only use the one from the personal
profile and not any from the global profile, the tshark command line,
or any other source."
Just mark the option as obsolete.
Related to #11470
|
|
|
|
Before:
Filter:
frame[:2] == $@frame[:2]
Error: Range is not supported for entity @frame <FT_BYTES>
frame[:2] == $@frame[:2]
^~~~~~~
After:
Filter:
frame[:2] == $@frame[:2]
Instructions:
0000 READ_TREE frame -> R0
0001 IF_FALSE_GOTO 7
0002 SLICE R0[0:2] -> R1
0003 READ_REFERENCE ${@frame} -> R2
0004 IF_FALSE_GOTO 7
0005 SLICE R2[0:2] -> R3
0006 ANY_EQ R1 == R3
0007 RETURN
|
|
Allow references without braces, for a less cluttered syntax:
Filter:
frame.number > $frame.number
Instructions:
0000 READ_TREE frame.number -> R0
0001 IF_FALSE_GOTO 5
0002 READ_REFERENCE ${frame.number} -> R1
0003 IF_FALSE_GOTO 5
0004 ANY_GT R0 > R1
0005 RETURN
The original syntax of ${reference} came from macros but the
braces don't add much. In any case they are still allowed.
|
|
|
|
Allow functions to be tested for "existence". This is in fact
not an existence test but a truthiness test for the function
return value.
|
|
- Change the DENIED test from testing denying the write, to denying the wrong data type.
- Add a SETTER test to see the value change. This also requires incrementing the number of expected SETTER tests.
|
|
It's more compact than "bitwise_and" and inspired by C.
|
|
Extend the time arithmetic to support multiplication and division
with floating point numbers.
In this case the multiplier/divisor is parsed according to its
lexical number type.
Fixes #19150.
|
|
Add a lexical type for numbers, consisting of integers and floats.
Because the float is represented internally as an IEEE 754 double
and that is a lossy representation, we still need to parse numbers
from strings sometimes.
One sematic change instroduced is that integers on the RHS of a
relation with bytes are interpreted as decimals instead of
hexadecimals without a 0x prefix in the representation.
This is a net win, in line with the goal of being explicit
writing literal values in display filter expressions.
|
|
The multiplication of two time values is not well-defined,
because time is represented internally as a vector.
Add a scalar multiplication/division for time values
using integer numbers. The scalar multiplier must appear
on the RHS of the operation. (This limitation mat be
removed in the future.)
This is useful to compare relative time values. The
operation is also allowed for absolute date and time values
because it is mathematically consistent but that is
probably less useful in practice.
Related to #19150.
|
|
The display filter language is statically typed, but the syntax
is not declarative. This means every literal type must be
inferred at compile time. This is not trivial if we have a constant
expression on the left-hand side.
To improve the flexibility and generality of the compiler we add a
look-ahead type resolution step when the type of a value cannot be
immediately inferred.
This simplifies the code, improves some error messages and enables the
correct handling of some expressions that were not supported before,
such as "1 == frame.number".
|
|
The tshark man page and help claims to support the --read-file,
--read-filter, and --display-filter long options. Make it so.
Programs have to dissect in order to run filters, so add the
filtering options to dissect_opts.h, which Wireshark, tshark,
and rawshark already include, so that they stay consistent
between applications.
Reading a file is a common activity that we might want to have
more (all?) command line tools support with the same syntax
eventually, so add that to clopts_common.h
rawshark doesn't distinguish between read filters and display
filters; to make things easy and consistent, make them synonymous
for rawshark.
Fix #19365
|
|
The pointer "s" is no longer valid when the warning is added.
Fix the use-after-free.
Add a test for the warning.
|
|
Before:
dftest 'tcp.checksum.status matches "good"'
Filter:
tcp.checksum.status matches "good"
Error: tcp.checksum.status (type=Unsigned integer (8 bits)) cannot participate in matches comparison.
tcp.checksum.status matches "good"
^~~~~~~~~~~~~~~~~~~
After:
$ dftest 'tcp.checksum.status matches "good"'
Filter:
tcp.checksum.status matches "good"
Instructions:
0000 READ_TREE tcp.checksum.status -> R0
0001 IF_FALSE_GOTO 5
0002 VALUE_STRING Vals:0x0x7eff633f6f80(R0) -> R1
0003 IF_FALSE_GOTO 5
0004 ANY_MATCHES R1 matches good
0005 RETURN
|
|
We should represent JSON values in JSON,
such as booleans using true/false.
The changes in cdc8e2f5119a20e905b60d2afab66114a86b45c9
didn't actually fix anything for JSON output in that
regard, they just replace one string representation with
a different incorrect representation, so revert the part
affecting JSON to avoid inconveniencing users without
sufficient justification.
|
|
In .gitlab-ci.yml, add spaces and emoji to the build directory name. In
CMakeLists.txt, quote a path in a wrapper script. Quote our executable
and file paths where needed in our tests.
|
|
|
|
Try to run our merge requests in Debug mode to make sure the
test suite has the best chance of catching any issues.
This means we no longer have a Release build. This build was intended
to catch warnings that only appears with -O3 optimization level.
It seems to have been successful at that. If that becomes an issue
again we may want to add a new job with a Release type build.
This should help catch more runtime errors and assertions that
sometimes slip through the cracks (and may also be related with recent
changes to the build configuration) and should be easily caught in
the tests.
|
|
Add `hosts:` tap to `tap` method of `sharkd` which returns a list of
resolved hostnames found in the capture file, thus providing the same
information as `tsharks -z hosts`. The `hosts:` tap accepts an
optional list of comma-separated protocol names `ipv4`, `ip` (synonym
for `ipv4`) or `ipv6`. For example, `hosts:ipv4` returns only IPv4
hosts, `hosts:ipv6` returns only IPv6 hosts and `hosts:ipv4,ipv6`
returns both IPv4 and IPv6 hosts. If no protocol names are
given (i.e. if the request is just `host:`), both IPv4 and IPv6 hosts
are returned. In the response, IPv4 and IPv6 hosts are returned in
separate `ipv4_hosts` and `ipv6_hosts` arrays containing objects with
`name` and `addr` fields. Both arrays are sorted by the `name` field
of each object.
|
|
|
|
Instead of adding a TFS(&tfs_true_false) to every boolean
field, make it the default if "strings" is NULL.
This seems to match the already existing documentation:
If the Boolean field is to be displayed as "False" or "True", the
'strings' field would be set to NULL.
|
|
Booleans can have true_false value strings other
than "True" and "False". We still need to parse
those.
Fixes 4b6268c01eb8e8f4e96760a89552ca9672069c3f.
Related to #19351.
|
|
The file data field for HTTP is the Content. It is a stream of
octets, and doesn't have a defined encoding (unlike headers)
inside the HTTP dissector; encodings can be interpreted by
the appropriate media handle or other subdissector.
Don't treat the content as a string with ASCII encoding; make it
an FT_BYTES, like the UDP and TCP payload fields. Some content
types are text and might have an encoding, but others are simply
binary data.
Related to #19280
|
|
Add a dissector for the 802.1cb protocol R-TAGs. This displays the R-TAG
and continues with the contained sub-protocol. The dissector does not do
de-duplication, or even check for dropped packets.
|
|
Global name resolution tests on macOS require adding a 'hosts' file to
Contents/MacOS in the application bundle. Newer versions of Xcode appear
to leave this unwritable after we've created our application bundle. We
arguably shouldn't modify the application bundle anyway, so skip global
name resolution tests if we have an application bundle.
|
|
|