Skip to content

fix(js): probe SMBv1 support after SMB2 negotiation - #7430

Merged
Mzack9999 merged 1 commit into
devfrom
dwisiswant0/fix/js/probe-SMBv1-support-after-SMB2-negotiation
May 30, 2026
Merged

fix(js): probe SMBv1 support after SMB2 negotiation#7430
Mzack9999 merged 1 commit into
devfrom
dwisiswant0/fix/js/probe-SMBv1-support-after-SMB2-negotiation

Conversation

@dwisiswant0

@dwisiswant0 dwisiswant0 commented May 27, 2026

Copy link
Copy Markdown
Member

Proposed changes

ConnectSMBInfoMode only tried SMBv1 when SMB2/3
negotiation failed so dual-stack servers could
report SupportV1 as false even when SMBv1 was
enabled.

Run a same-target SMBv1 probe after successful
SMB2/3 negotiation and merge the support flag into
the returned log w/o changing the negotiated
SMB2/3 version.

Fixes #4832

Proof

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced SMB protocol negotiation reliability by improving version detection and connection handling.
  • Tests

    • Added comprehensive test coverage for SMB protocol version support detection.

Review Change Stack

`ConnectSMBInfoMode` only tried SMBv1 when SMB2/3
negotiation failed so dual-stack servers could
report `SupportV1` as false even when SMBv1 was
enabled.

Run a same-target SMBv1 probe after successful
SMB2/3 negotiation and merge the support flag into
the returned log w/o changing the negotiated
SMB2/3 version.

Fixes #4832

Signed-off-by: Dwi Siswanto <git@dw1.io>
@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 311d7b66-bf60-4552-9be5-260f8fb87eb5

📥 Commits

Reviewing files that changed from the base of the PR and between 0bc707a and dcafbe3.

📒 Files selected for processing (3)
  • pkg/js/libs/smb/smb.go
  • pkg/js/libs/smb/smb_private.go
  • pkg/js/libs/smb/smb_test.go

Walkthrough

The PR refactors SMB version detection to properly detect SMBv1 support. A new type abstraction and probe function are introduced to test SMBv1 capability after SMBv2/v3 negotiation succeeds. The main connection handler is simplified by centralizing dialing logic and integrating the new probe. Unit tests cover successful probes and error conditions.

Changes

SMBv1 Probe Detection

Layer / File(s) Summary
SMBv1 probe type and implementation
pkg/js/libs/smb/smb_private.go
smbInfoDialFunc type represents context-aware dialing for SMB. updateSMBv1Support probes SMBv1 support conditionally via injected dial function, updating SupportV1 flag when probe succeeds and preserving existing negotiated version details.
Connection flow and probe invocation
pkg/js/libs/smb/smb.go
connectSMBInfoMode centralizes TCP dialing with net.JoinHostPort, attempts SMBv2/v3 negotiation with guaranteed cleanup, invokes SMBv1 probe on success, and falls back to SMBv1 negotiation path on SMBv2/v3 failure.
SMBv1 probe test coverage and helpers
pkg/js/libs/smb/smb_test.go
Test helpers implement a fake SMBv1 probe server using net.Pipe with protocol-compliant responses. Tests verify updateSMBv1Support sets SupportV1 while preserving SMB2 negotiated version, and handles dial failures safely.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A probe hops in to check what's true,
SMBv1 or the v2 crew!
No more guessing, version's right,
Detection shines with proper light!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: probing SMBv1 support after SMB2 negotiation, which directly addresses the linked issue #4832.
Linked Issues check ✅ Passed The changes implement SMBv1 probing after successful SMB2/3 negotiation to accurately detect dual-stack servers, directly addressing issue #4832's requirement to report correct SMB versions.
Out of Scope Changes check ✅ Passed All changes are focused on the SMB detection logic refactoring and SMBv1 support probing; no unrelated modifications are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dwisiswant0/fix/js/probe-SMBv1-support-after-SMB2-negotiation

Comment @coderabbitai help to get the list of available commands and usage tips.

@dwisiswant0
dwisiswant0 requested a review from Mzack9999 May 29, 2026 11:08
@Mzack9999
Mzack9999 merged commit bbc010b into dev May 30, 2026
16 of 19 checks passed
@Mzack9999
Mzack9999 deleted the dwisiswant0/fix/js/probe-SMBv1-support-after-SMB2-negotiation branch May 30, 2026 07:16
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.

Javascript SMB module not detecting correct SMB version

2 participants