fix(binary): treat .DS_Store files as binary by extension

What does this MR do and why?

The indexer's binary extension list has a .DS_Store entry that never matched: the lookup lower-cases the extension, and the entry was mixed case. A .DS_Store file was therefore indexed as text unless its first 8 KiB held a NUL byte. Files written by Finder do start with NUL bytes, so only other .DS_Store files were affected. The entry is now lower case, and a new test keeps every entry that way.

Testing

The two new .DS_Store cases without a NUL byte and the new lower-case test fail on main and pass with the change. The binary package and the indexer packages that call it in both modes pass with -race on Go 1.26.7 and 1.27.3, and make golangci-lint in the CI image reports 0 issues.

Performance analysis

One map key changes; the lookup does the same work.

  • This merge request does not introduce a performance regression. If a regression is expected, explain why.
Details: cause, behavior change, test output and checks

Cause. In internal/shared/binary/binary.go the entry is ".DS_Store": true (L69), while isBinaryExtension looks up strings.ToLower(filepath.Ext(filename)) (L91-94). For .DS_Store and assets/.DS_Store, filepath.Ext returns .DS_Store, which becomes .ds_store and misses the key, so DetectBinary falls through to the NUL byte check. The key and the lower-casing lookup came in together with !681 (merged); every other key in the list is already lower case.

Where it applies. DetectBinary is called by BuildBlob in advanced mode, which then indexes the blob with empty content instead of its text (blob.go#L99), and by BulkIndex in chunk mode, which skips the file (chunk.go#L303).

Change. The key is now ".ds_store", the comment above the map says keys must be lower case, TestDetectBinary gains a .DS_Store case at the root and one in a subdirectory (content Bud1 metadata, no NUL byte), and TestBinaryExtensionsAreLowerCase fails on any key with upper-case letters, so a new entry cannot repeat this.

Behavior change. Any file whose extension is .ds_store in any case (x.DS_STORE included) is now treated as binary whatever its content; a name without the leading dot such as DS_Store is not. A 6148-byte .DS_Store sample in Finder's format starts with 00 00 00 01 42 75 64 31 (Bud1), and DetectBinary(".DS_Store", sample) returns true both before and after this change, through the NUL byte check.

The new tests with binary.go as it is on b9a82748 (go test -count=1 -run 'TestDetectBinary|TestBinaryExtensionsAreLowerCase' ./internal/shared/binary/, Error Trace and Diff lines left out):

--- FAIL: TestBinaryExtensionsAreLowerCase (0.00s)
    binary_test.go:150:
        	Error:      	Not equal:
        	            	expected: ".ds_store"
        	            	actual  : ".DS_Store"
        	Test:       	TestBinaryExtensionsAreLowerCase
        	Messages:   	isBinaryExtension lower-cases the extension, so a key with upper-case letters never matches
--- FAIL: TestDetectBinary (0.00s)
    --- FAIL: TestDetectBinary/ds_store_file_in_subdirectory (0.00s)
        binary_test.go:141:
            	Error:      	Not equal:
            	            	expected: true
            	            	actual  : false
    --- FAIL: TestDetectBinary/ds_store_file_without_null_bytes (0.00s)
        binary_test.go:141:
            	Error:      	Not equal:
            	            	expected: true
            	            	actual  : false
FAIL
FAIL	gitlab.com/gitlab-org/gitlab-elasticsearch-indexer/internal/shared/binary	0.003s

Every existing case passes before and after. With the change, the same tests pass with -race on Go 1.26.7 and 1.27.3. Putting the mixed-case key back, writing it as .DS_STORE, or removing the entry each makes the new tests fail again.

Other checks with the change:

  • make golangci-lint in golangci/golangci-lint:v2.11.3 with libicu-dev, as the lint:golangci job runs it: 0 issues.
  • go vet over every package, ./scripts/check-go-format.sh, goimports -l on the package, go mod tidy (no change) and make build.
  • go test -race -count=1 on ./internal/shared/binary/..., ./internal/mode/advanced/indexer/... and ./internal/mode/chunk/... with Go 1.26.7 and 1.27.3, and on the nested third_party/icu module with Go 1.26.7.
  • make test without Elasticsearch or Gitaly: every package passes except TestBulkSizeTracking, which needs a running Elasticsearch and fails the same way on b9a82748 without this change.
  • commitlint with the project's .commitlintrc.yml on the commit and on this title: 0 problems.

Not run: the integration tests that need make test-infra. None of the six gitlab-test commits they index has a .DS_Store file, so their results cannot depend on this change.

Environment: Linux amd64, Go 1.26.7 (from .tool-versions) and 1.27.3, libicu-dev 76.1.

Merge request reports

Loading
Loading