Skip to content
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

Fix DATA RACE as a result of changing ber's module global variable for fuzz tests #473

Merged
merged 1 commit into from Nov 21, 2023

Conversation

t2y
Copy link
Contributor

@t2y t2y commented Nov 20, 2023

I investigated why DATA RACE occurred.

WARNING: DATA RACE
Write at 0x000000c3c628 by goroutine 38:
  github.com/go-ldap/ldap.FuzzParseDN()
      /home/runner/work/ldap/ldap/fuzz_test.go:14 +0x32
  testing.fRunner()
      /opt/hostedtoolcache/go/1.18.10/x64/src/testing/fuzz.go:700 +0x168
  testing.runFuzzTests.func1()
      /opt/hostedtoolcache/go/1.18.10/x64/src/testing/fuzz.go:5[20](https://github.com/go-ldap/ldap/actions/runs/6781841980/job/18432888679#step:5:21) +0x47

Fuzz tests also run as unit tests, so the module global variable shouldn't change in a test function.

ber.MaxPacketLengthBytes = 65536

I fix this conflict by adding TestMain(), which handles a setup process before all tests. Now, all tests run with ber.MaxPacketLengthBytes limitation. Does it make sense?

See also #472.

References

… of changing ber's module global variable for fuzz tests go-ldap#472
@johnweldon
Copy link
Member

Thank you @t2y
We do want this in the v3 folder eventually, but for now this will get you past the CI errors.

@johnweldon johnweldon merged commit ef0e538 into go-ldap:master Nov 21, 2023
20 checks passed
@t2y t2y deleted the fix-fuzz-test branch November 21, 2023 00:07
gustavoluvizotto pushed a commit to gustavoluvizotto/ldap-fork that referenced this pull request Apr 11, 2024
… of changing ber's module global variable for fuzz tests go-ldap#472 (go-ldap#473)
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.

None yet

2 participants