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

Websockets 2695 v18 #10842

Closed
wants to merge 7 commits into from
Closed

Conversation

catenacyber
Copy link
Contributor

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/2695

Describe changes:

SV_BRANCH=OISF/suricata-verify#1571

#10409 rebased after merge of #10819 (now calling AppLayerParserRegisterLogger from rust/src/websocket/websocket.rs)

if no config option is found,
as is done for udp

Ticket: 6304
Including the one for websocket over HTTP/2
port is used in AppLayerProtoDetectProbingParserPort
and not in AppLayerProtoDetectProbingParserElement
As for WebSocket which is detected only by protocol change.
When there is a protocol change, and a specific protocol is
expected, like WebSeocket, always run it, no matter the port.
@catenacyber catenacyber force-pushed the websockets-2695-v18 branch from a88eb11 to 7b346bb Compare April 14, 2024 20:05
Copy link

codecov bot commented Apr 14, 2024

Codecov Report

Attention: Patch coverage is 86.47059% with 69 lines in your changes are missing coverage. Please review.

Project coverage is 82.76%. Comparing base (784ce30) to head (7b346bb).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10842      +/-   ##
==========================================
- Coverage   82.83%   82.76%   -0.07%     
==========================================
  Files         913      918       +5     
  Lines      246847   247298     +451     
==========================================
+ Hits       204474   204677     +203     
- Misses      42373    42621     +248     
Flag Coverage Δ
fuzzcorpus 64.14% <58.03%> (-0.16%) ⬇️
suricata-verify 62.15% <78.43%> (+0.06%) ⬆️
unittests 62.25% <21.96%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 20061

@victorjulien victorjulien added this to the 8.0 milestone Apr 15, 2024
@victorjulien
Copy link
Member

32 bit ARM:

   Compiling suricata v8.0.0-dev (/builds/inliniac/suricata-ci/suricata/rust)
error[E0277]: the trait bound `u64: ToUsize` is not satisfied
   --> src/websocket/parser.rs:75:33
    |
75  |     let (i, payload_raw) = take(payload_len)(i)?;
    |                            ---- ^^^^^^^^^^^ the trait `ToUsize` is not implemented for `u64`
    |                            |
    |                            required by a bound introduced by this call
    |
    = help: the following other types implement trait `ToUsize`:
              u16
              u32
              u8
              usize
note: required by a bound in `nom::bytes::streaming::take`
   --> /builds/inliniac/suricata-ci/suricata/rust/vendor/nom/src/bytes/streaming.rs:416:6
    |
416 |   C: ToUsize,
    |      ^^^^^^^ required by this bound in `nom::bytes::streaming::take`
For more information about this error, try `rustc --explain E0277`.
error: could not compile `suricata` due to previous error
make[1]: *** [all-local] Error 101
Makefile:548: recipe for target 'all-local' failed
make[1]: Leaving directory '/builds/inliniac/suricata-ci/suricata/rust'
Makefile:500: recipe for target 'all-recursive' failed
make: *** [all-recursive] Error 1

@victorjulien victorjulien removed this from the 8.0 milestone Apr 15, 2024
@catenacyber
Copy link
Contributor Author

32 bit ARM:

   Compiling suricata v8.0.0-dev (/builds/inliniac/suricata-ci/suricata/rust)
error[E0277]: the trait bound `u64: ToUsize` is not satisfied
   --> src/websocket/parser.rs:75:33
    |
75  |     let (i, payload_raw) = take(payload_len)(i)?;
    |                            ---- ^^^^^^^^^^^ the trait `ToUsize` is not implemented for `u64`
    |                            |
    |                            required by a bound introduced by this call
    |
    = help: the following other types implement trait `ToUsize`:
              u16
              u32
              u8
              usize
note: required by a bound in `nom::bytes::streaming::take`
   --> /builds/inliniac/suricata-ci/suricata/rust/vendor/nom/src/bytes/streaming.rs:416:6
    |
416 |   C: ToUsize,
    |      ^^^^^^^ required by this bound in `nom::bytes::streaming::take`
For more information about this error, try `rustc --explain E0277`.
error: could not compile `suricata` due to previous error
make[1]: *** [all-local] Error 101
Makefile:548: recipe for target 'all-local' failed
make[1]: Leaving directory '/builds/inliniac/suricata-ci/suricata/rust'
Makefile:500: recipe for target 'all-recursive' failed
make: *** [all-recursive] Error 1

Tried to fix that in #10849 by limiting the payload taken to u32...

Only your QA checks 32-bit builds ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants