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.

Sign up for GitHub

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

SNOW-892592: Fix panic when SF_OCSP_RESPONSE_CACHE_SERVER_ENABLED=false #883

Closed
wants to merge 1 commit into from
Closed

SNOW-892592: Fix panic when SF_OCSP_RESPONSE_CACHE_SERVER_ENABLED=false #883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions ocsp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -716,11 +716,12 @@ func overrideCacheDir() {

// initOCSPCache initializes OCSP Response cache file.
func initOCSPCache() {
ocspResponseCache = make(map[certIDKey][]interface{})
ocspResponseCacheLock = &sync.RWMutex{}

if strings.EqualFold(os.Getenv(cacheServerEnabledEnv), "false") {
return
}
ocspResponseCache = make(map[certIDKey][]interface{})
ocspResponseCacheLock = &sync.RWMutex{}

logger.Infof("reading OCSP Response cache file. %v\n", cacheFileName)
f, err := os.OpenFile(cacheFileName, os.O_CREATE|os.O_RDONLY, readWriteFileMode)
Expand Down
18 changes: 18 additions & 0 deletions ocsp_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,3 +533,21 @@ func TestInitOCSPCacheFileCreation(t *testing.T) {
t.Error(err)
}
}

func TestCacheServerDisabled(t *testing.T) {
_ = os.Setenv(cacheServerEnabledEnv, "false")

if _, err := buildSnowflakeConn(context.Background(), Config{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can extract building sf conn outside of the if? Or at least Config? Now it's not very readable.

Account: "testaccount",
User: "testuser",
Password: "testpassword",
Host: "testaccount.us-east-1.snowflakecomputing.com",
}); err != nil {
t.Error(err)
}

ocspURL := os.Getenv(cacheServerURLEnv)
if ocspURL != "" {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I understand it. According to the first line in the test, you just set an env variable and now you read another one. I believe this just reads env variable, there is no driver logic. The only reason why this could work is that buildSnowflakeConn removes this env var, which seems unlike. But if this should work, maybe we should enrich the test with setting cacheServerUrlEnv at the beginning of the test?

t.Fatalf("cache server is disabled. Expected empty url but got %v ", cacheServerURLEnv)
}
}