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.003sEvery 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-lintingolangci/golangci-lint:v2.11.3withlibicu-dev, as thelint:golangcijob runs it:0 issues.go vetover every package,./scripts/check-go-format.sh,goimports -lon the package,go mod tidy(no change) andmake build.go test -race -count=1on./internal/shared/binary/...,./internal/mode/advanced/indexer/...and./internal/mode/chunk/...with Go 1.26.7 and 1.27.3, and on the nestedthird_party/icumodule with Go 1.26.7.make testwithout Elasticsearch or Gitaly: every package passes exceptTestBulkSizeTracking, which needs a running Elasticsearch and fails the same way on b9a82748 without this change.- commitlint with the project's
.commitlintrc.ymlon 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.