Skip to content

Fixes for NMB Protocol #1970

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

Merged
merged 4 commits into from
Jul 14, 2025
Merged

Fixes for NMB Protocol #1970

merged 4 commits into from
Jul 14, 2025

Conversation

NtAlexio2
Copy link
Contributor

This PR fixes some issues in NetBIOS protocol:

  1. Add missing field to marshaling process in NBNodeStatusResponse() structure. In order to RFC1002, the NODE STATUS RESPONSE structure contains statistics field in the end.
  2. Some code improvements in parameters
  3. Use mocks in nmb tests, to work locally regardless of remote (expired) environment(s)

@anadrianmanrique anadrianmanrique added the in review This issue or pull request is being analyzed label May 29, 2025
@anadrianmanrique anadrianmanrique self-assigned this May 29, 2025
@anadrianmanrique
Copy link
Collaborator

Hi @NtAlexio2. Thanks for the PR
Regarding test changes, is there any reason to mock a remote test? Remote tests are intended to test functionality against a real target. In case where a specific functionality should be tested locally we do it in local unittest, as a separate test.

@anadrianmanrique anadrianmanrique added medium Medium priority item and removed in review This issue or pull request is being analyzed labels Jun 5, 2025
@NtAlexio2
Copy link
Contributor Author

@anadrianmanrique There is no specific reason. should I rebase to b3fc47f? and also move tests to local unittest?

@anadrianmanrique
Copy link
Collaborator

@NtAlexio2 yes please. Thanks!

@anadrianmanrique anadrianmanrique added the waiting for response Further information is needed from people who opened the issue or pull request label Jun 23, 2025
@NtAlexio2
Copy link
Contributor Author

@anadrianmanrique done

@anadrianmanrique
Copy link
Collaborator

Thank you. Merging now

@anadrianmanrique anadrianmanrique merged commit bb69bf3 into fortra:master Jul 14, 2025
8 checks passed
@NtAlexio2 NtAlexio2 deleted the nmb branch July 14, 2025 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium Medium priority item waiting for response Further information is needed from people who opened the issue or pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants