Fix a couple of edge cases#2
Open
smcintyre-r7 wants to merge 4 commits intoZ6543:win9x-smb1-supportfrom
Open
Conversation
The cfddc48 follow-up retyped OpenAndxRequest::ParameterBlock's search_attributes and file_attributes from uint16 to the smb_file_attributes BitField, but Tree#_open_andx still assigns Integer literals to them. BinData rejects this with "undefined method 'each_pair' for an instance of Integer", which breaks every Win9x code path that opens a file or pipe (e.g. Tree#open_file, Tree#open_pipe, and Client#net_share_enum_all when SMB1 is in use against a non-NT server). Use BitField#read with the packed mask, which round-trips to the same wire bytes (0x0016 -> directory|system|hidden, 0x0020 -> archive) without going through BinData::Struct#do_assign. Add four #open_file specs covering the OPEN_ANDX dispatch path that exercise the request builder and FID/size handling against a stubbed response. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Win9x-era SMB1 servers (observed on Windows ME) pack trans2_parameters directly after byte_count with no 4-byte-alignment pad, and signal this by sending word_count=10 and a parameter_offset that points at the byte immediately following byte_count. BinData's Trans2::DataBlock#pad1_length unconditionally inserts the NT-style pad on read, so trans2_parameters and trans2_data both arrive shifted by the pad width — eos/sid come back garbled and the buffer is truncated, leaving #results with 0 entries. Detect the mismatch by comparing parameter_block.data_count to the bytes BinData actually read into trans2_data.buffer. When they differ and the raw response has enough bytes at the server-reported parameter_offset / data_offset, re-slice both sections from the raw response and hand them to the list loop — the effective trans2_parameters drive the eos / sid termination condition, and the trans2_data bytes feed #results through its new buffer: kwarg. Follows the same shape as the e243f02 fix for Rap::NetShareEnum. Regression test synthesizes a word_count=10, pad1=0 FIND_FIRST2 response carrying three SMB_INFO_STANDARD entries (".", "..", "FLAG.TXT.txt") matching the bytes captured from a live Windows ME target; before the fix BinData's buffer is 2 bytes short and 0 entries parse, after the fix all three round-trip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rap::NetShareEnum#net_share_enum returned :type as the raw Integer from share_info_1.shi1_type (e.g. 0x0003), while the SRVSVC equivalent in Dcerpc::Srvsvc#net_share_enum_all returned it as a pipe-joined string (e.g. "IPC|SPECIAL"). Consumers that want to enumerate shares without caring which transport carried the answer had to special-case the two APIs. Format the RAP type the same way: look up the low-byte base code against a local SHARE_TYPES table (MS-RAP 2.5.14 only defines DISK / PRINTER / DEVICE / IPC, so it's smaller than the SRVSVC variant), then append SPECIAL / TEMPORARY modifiers when those bits are set. Unknown base codes render as UNKNOWN(0xXXXX) so they round-trip visibly rather than getting silently dropped. Note: RAP share_type is 16 bits, so STYPE_SPECIAL and STYPE_TEMPORARY are 0x8000 / 0x4000 (not the 32-bit 0x80000000 / 0x40000000 values SRVSVC uses). Observed Windows ME responses do not set either bit, but other servers may. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The server-reported-offset re-slice introduced in e422e10 lived inline on Tree#list, hardcoded to FindFirst2ResponseTrans2Parameters and only reachable through that one call site. Other Trans2 subcommands (OPEN2, QUERY_FILE_INFORMATION, QUERY_FS_INFORMATION, etc.) will need the same workaround if Win9x compatibility gets widened beyond directory listing, and duplicating the same four-line detect-and-slice block at each caller is exactly the kind of drift that makes Win9x quirks painful to chase later. Move the logic to RubySMB::SMB1::Packet::Trans2::Win9xFraming, a tiny mixin with a single #win9x_trans2_overrides(raw_response) method. The mixin consumes only fields that every Trans2 response already has (parameter_block.{parameter_offset, parameter_count, data_offset, data_count}; data_block.{trans2_parameters, trans2_data.buffer}), and the parameter-struct type is discovered from data_block.trans2_parameters.class so the same method works across subcommands without per-caller hardcoding. FindFirst2Response includes the mixin; Tree#list now calls response.win9x_trans2_overrides(raw_response) instead of the private helper it carried before. The private Tree#win9x_trans2_overrides is gone. New spec covers the mixin directly (NT-style no-op, zero data_count, Win9x-era slice for both parameters and data, truncated raw response) using FindFirst2Response as the concrete host so the field layouts stay real. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes the three issues I ran into while testing this. The first two commits resulted in exceptions along the happy path of the test harness and the third addresses a discrepancy in the shape of the share data where the type was previously an integer and not a string. We'll eventually abstract away the share enumeration, so having the same data type will be helpful. It also moves the win9x framing work around into a mixin so it'll be reusable in the future.